Skip to content

Conversation

@nolash
Copy link
Contributor

@nolash nolash commented Jan 26, 2017

fixes #3444
fixes #3494
networkid override

Added comments to explain why test against 0 appears twice

  • Command line overrides saved config, saved config overrides system default

fixes #3476
bzzr get with path

Finally a hopefully clean commit for this PR
Added check for empty path to avoid SIGSEGV in path parser and resolver
Added requested tests for empty path and non-existing manifest.
However signature for StartHTTPServer had changed.
Now it's hacked as so:

StartHttpServer(api.API, &Server{Addr: "127.0.0.1:8504", CorsString: ""})
  • Parse url before resolve when path and ENS is supplied, example
  • swarm/api/http proxy server test for retrieval of subpath through get
  • Removed nil entry assignment on subtrie leaf in recursive key retrieval
  • Cleaned up path-or-no-path condition in proxy server get handler
  • swarm: processed with gofmt refers to lash/go-ethereum@90daa7a
  • swarm: Added public access method Parse alias to parse
  • swarm: processed with gofmt References nolash/go-ethereum@2ec3fd7
  • Rename parse to Parse, removed alias

@mention-bot
Copy link

@nolash, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fjl, @zelig and @maran to be potential reviewers.

@nolash
Copy link
Contributor Author

nolash commented Jan 26, 2017

@zelig don't know what's up with travis-ci but the code passes all tests here locally.

Copy link
Contributor

@zelig zelig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very good, thanks for taking the trouble.
Two tiny things:

  • explicit field names on imported struct in server_test
  • please consider removing debug log lines in server_test
  • remove commented out code and import lines

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go vet complains on travis about anonymous fields for imported struct types. You need to explicitly specify the field names here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try not to leave commented out bits in the code

fixes ethereum#3444
fixes ethereum#3494
networkid override

Added comments to explain why test against 0 appears twice

* Command line overrides saved config, saved config overrides system default

---

fixes ethereum#3476
bzzr get with path

Finally a hopefully clean commit for this PR
Added check for empty path to avoid SIGSEGV in path parser and resolver
Added requested tests for empty path and non-existing manifest.
However signature for StartHTTPServer had changed.
Now it's hacked as so:

	StartHttpServer(api.API, &Server{Addr: "127.0.0.1:8504", CorsString: ""})

* Parse url before resolve when path and ENS is supplied, example
* swarm/api/http proxy server test for retrieval of subpath through get
* Removed nil entry assignment on subtrie leaf in recursive key retrieval
* Cleaned up path-or-no-path condition in proxy server get handler
* swarm: processed with gofmt refers to lash/go-ethereum@90daa7a
* swarm: Added public access method Parse alias to parse
* swarm: processed with gofmt References 2ec3fd7
* Rename parse to Parse, removed alias
Copy link
Contributor

@zelig zelig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks you @nolash brilliant PR

@zelig zelig added review and removed in progress labels Jan 27, 2017
@@ -0,0 +1,117 @@
package http
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copyright header is missing.

@karalabe karalabe merged commit 1c140f7 into ethereum:master Jan 30, 2017
@obscuren obscuren removed the review label Jan 30, 2017
@nolash nolash deleted the bzzpathfix_real5 branch February 5, 2017 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

swarm: bzzr: doesn't properly parse GET requests swarm config file options take precedence over command line arguments

6 participants