Open
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.
Summary
This PR switches off from
HTTPServertoNettyto fix several vulnerabilities while still remaining efficient. The motivation for this PR is to allow for bigger servers to use the webserver without worrying about malicious users trying to trojan horse the server. Additionally closes #150.Notable Changes
config.yml: ReplacesUploadService.DisplayURL,UploadService.WebServer.Host, andUploadService.WebServer.PortwithUploadService.BaseURI.BaseURIuses an HTTP URI, which naturally includes all of the above information while also giving the user more control over what the server is listening on and more compatibility with other web services hosted on the same domain name.ImageUploadManager: Switched away fromHttpServertoNetty, fixing several vulnerabilities and including several optimizations:HttpServerwas fairly functional,Nettyhas a lot of built in security checks to prevent common malicious attacks. It's also more compliant with the HTTP protocol, which would prevent edge case bugs that weren't covered by the manual implementation of multipart requests withinPOST /upload.Files.probeContentTypeand setting the proper secure headers.Notes
Untested! While I was able to verify the code compiles, I wasn't able to build the jar file due to a dependency on
ImageFrame-Parent, which I wasn't able to find. If you could answer my question on your Discord, that'd be greatly appreciated and I'd be happy to test the changes myself.If you have any questions, you're welcome to drop them below. If you have nitpicks on code style or implementation decisions, I encourage you to merge the PR and then push your changes afterwards.