Repository navigation
Confine upload_download paths to the configured os.Root directory - #224
Merged
Merged
Conversation
This file contains hidden or 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
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.
The
upload_downloadexample shows how a Goa HTTP service (updown) streams multipart file uploads (POST /upload/{*dir}) and file downloads (GET /download/{*filename}) without buffering whole request or response bodies in memory. Previously, both handlers joined the decoded URL wildcard parameter onto the configured base directory withfilepath.Joinwithout checking whether the resulting path stayed inside that base directory. In the HTTP tests, a request such asPOST /upload/..%2FoutsideorGET /download/..%2Fsecret.txtresolved outsidepublic/, allowing callers who copied the example to create directories (with mode0777), overwrite files, or read files outside the intended directory.Uploadalso deferred closing each created file inside the multipart loop—keeping every part's file descriptor open until the request finished—and accepted multipart parts with empty,., or..file names.Opening an
*os.Roothandle incmd/upload_download/main.goand passing it intoNewUpdownconfines every directory creation, file write, and file read to the configured base directory at the OS level, including across symbolic links inside that directory.UploadandDownloadfirst validate and convert slash-separated URL paths withfilepath.Localizeso parent-directory segments (..) fail before creating any intermediate directories on disk, while still allowing trailing slashes on upload directory URLs.design/design.godeclaresinvalid_file_path(400 Bad Request) onuploadalongsidedownload's existing404 Not Foundmapping so invalid, non-directory, or escaping paths return400/404while permission and storage failures continue to return500(internal_error).Uploadcreates missing subdirectories with mode0750, rejects multipart parts whose file name is empty,.,.., or contains path separators, and closes each destination file before advancing to the next part.HTTP handler tests in
upload_download/updown_test.goexercise nested and root-directory multipart uploads, trailing slashes,0750directory modes, URL-encoded..traversal, symlinks pointing outside the base directory, existing regular-file collisions, read-only directory500responses, invalid part file names, malformedContent-Typeheaders, and file downloads.