Fix tests and make path handling more robust #23
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The tests did not have a correct HMAC for the example file, which causes them
to fail to verify the token correctly.
The new HMAC was generated with the following Python REPL session:
>>> import hmac
>>> h = hmac.new(b'mysecret', digestmod='sha256')
>>> h.update(b'thomas/abc/catmetal.jpg 23026')
>>> h.hexdigest()
'7b8879e2d1c733b423a70cde30cecc3a3c64a03f790d1b5bcbb2a6aca52b477e'
In addition, the filer returns 201 Created when a file is
created, not 200 Okay, which is consistent with the behaviour of
https://github.com/horazont/xmpp-http-upload
Previous versions of this code relied on the upload subdir configuration parameter being set to a relative pathname, and then prepending a leading slash to make it absolute. This is a little fragile, especially where the config parameter is specified with a leading slash, so replace this with path.Join to get consistent behaviour in both of these cases. The store directory's pathname is also assumed to be given with a trailing slash, with the request filename being simply appended to this; use filepath.Join to get consistent behaviour here too, in case the trailing slash is not provided. Note that http.Server performs path canonicalisation and sanitisation on the request URL path internally before invoking the specified handler function, so this doesn't need to be done manually.
This fixes a regression introduced in the path handling safety cleanup commit. http.ServeMux needs a trailing slash to detect rooted subtrees instead of individual file paths.
|
Thanks for your contribution to code quality, @sysvinit ! I'll check it out in the next few days and merge it into develop :-) |
|
Thanks for merging! I've got a couple of other changes and fixes which I'll clean up and then open separate PRs for. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These commits make the tests work again, and use the
pathandpath/filepathlibraries for path manipulation. This means the file data path doesn't need to have a trailing slash in order to work correctly, and that the upload sub directory can be given as an absolute path with a leading slash. Further details are in the commit messages.