Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting "deep" config options doesn't work #1561

Closed
seidtgeist opened this issue Aug 10, 2015 · 11 comments
Closed

Setting "deep" config options doesn't work #1561

seidtgeist opened this issue Aug 10, 2015 · 11 comments
Assignees
Labels
kind/bug A bug in existing code (including security flaws) topic/commands Topic commands

Comments

@seidtgeist
Copy link
Contributor

I consulted the new CORS documentation in ipfs daemon --help and found this

You can setup CORS headers the same way:

ipfs config --json API.HTTPHeaders.Access-Control-Allow-Origin '["*"]'
ipfs config --json API.HTTPHeaders.Access-Control-Allow-Methods '["PUT", "GET", "POST"]'
ipfs config --json API.HTTPHeaders.Access-Control-Allow-Credentials '["true"]'

However, when I tried it I got an error at first.

Using ipfs config show I saw that API.HTTPHeaders is set to null, so I set it to an empty object like this:

ipfs config --json API.HTTPHeaders '{}'

Afterwards I was also able set properties within the API.HTTPHeaders object.

I would have expected the ipfs config --json command to automatically create objects along a valid property path.

@whyrusleeping whyrusleeping added kind/bug A bug in existing code (including security flaws) topic/commands Topic commands labels Aug 10, 2015
@jbenet
Copy link
Member

jbenet commented Aug 11, 2015

I would have expected the ipfs config --json command to automatically create objects along a valid property path.

@ehd yeah, it should. interesting... can you repro with a test case? we should add it to test suite if so

@seidtgeist
Copy link
Contributor Author

@jbenet Yeah, gladly. Should I add it in this test?

I also found this test in user-config which sets a deep property successfully. Lo and behold, maybe a difference between null and not defined in the code that checks deep property access?

One more question: Where do I put the ipfs binary in order to run the tests? From the README I couldn't find out how to build ipfs in a way that makes it work within the test harness command from .travis.yml.

@jbenet
Copy link
Member

jbenet commented Aug 11, 2015

@ehd yeah that test works.

Where do I put the ipfs binary in order to run the tests? From the README I couldn't find out how to build ipfs in a way that makes it work within the test harness command from .travis.yml.

just run make. it should build its own version and place it in test/bin/ipfs

seidtgeist added a commit to seidtgeist/go-ipfs that referenced this issue Aug 12, 2015
This adds a failing for the case described in ipfs#1561
@seidtgeist
Copy link
Contributor Author

@jbenet seidtgeist@c6d1413 adds test cases for:

  • config set a path with a parent that is not defined: Passes
  • config set a path with a parent that is null (like setting a property inside API.HTTPHeaders & Gateway.HTTPHeaders): Fails

@jbenet
Copy link
Member

jbenet commented Aug 13, 2015

thanks @ehd!

@jbenet
Copy link
Member

jbenet commented Aug 13, 2015

@rht, @whyrusleeping, @Heems can one of you pick this up this week?

@rht
Copy link
Contributor

rht commented Aug 13, 2015

Waiting for merge on @ehd's commit before this is rebased and merged: #1570

@seidtgeist
Copy link
Contributor Author

thanks! should i open a PR or something to get the above commit merged?

@rht
Copy link
Contributor

rht commented Aug 13, 2015

@ehd uh huh

@seidtgeist
Copy link
Contributor Author

@rht I opened a PR pulling my commit into your fork so master doesn't get a failing test merged in. Please let me know if you want me to do something else instead.

rht pushed a commit to rht/go-ipfs that referenced this issue Aug 13, 2015
This adds a failing for the case described in ipfs#1561
rht pushed a commit to rht/go-ipfs that referenced this issue Aug 13, 2015
This adds a failing for the case described in ipfs#1561

License: MIT
Signed-off-by: Stephan Seidt <[email protected]>
@seidtgeist
Copy link
Contributor Author

high 🙏🏻 five. Thank you.

rht pushed a commit to rht/go-ipfs that referenced this issue Aug 15, 2015
This adds a failing for the case described in ipfs#1561

License: MIT
Signed-off-by: Stephan Seidt <[email protected]>
kbala444 pushed a commit to kbala444/go-ipfs that referenced this issue Aug 15, 2015
This adds a failing for the case described in ipfs#1561

License: MIT
Signed-off-by: Stephan Seidt <[email protected]>
@ajnavarro ajnavarro mentioned this issue Aug 24, 2022
72 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws) topic/commands Topic commands
Projects
None yet
Development

No branches or pull requests

4 participants