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.

Sign up for GitHub

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

Jump to bottom

Serve compressed files if already exist #852

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Serve compressed files if already exist #852

wants to merge 2 commits into from

Conversation

De-Panther
Copy link

@De-Panther De-Panther commented Feb 6, 2023 •
edited

If file is already compressed (ends with .gz or .br), no need to add those file extensions.
Sometime files are already compressed and needs to serve with the correct header.
In some cases engines like Unity output some of the files as compressed and others not.

This change allows to serve already compressed files even if Serve GZIP Files or Serve Brotli Files are false.

I'm not sure how to write a test, or if this change should be documented.
It seems to me like an obvious outcome when serving a .gz or .br file. So I think it's a bug fix rather than a feature or a change in a feature.

Relevant issues

Resolves #844
Resolves #780

Contributor checklist
  • Provide tests for the changes (unless documentation-only)
  • Documented any new features, CLI switches, etc. (if applicable)
    • Server --help output
    • README.md
    • doc/http-server.1 (use the same format as other entries)
  • The pull request is being made against the master branch
Maintainer checklist
  • Assign a version triage tag
  • Approve tests if applicable

@De-Panther
Copy link
Author

Apparently there were already tests for gzip and brotli, and my first commit broke the Content-Type tests.
I moved the defaultType to the start of the process right after the options are parsed.
Also moved the MIME lookup to right after we get the file, which made redundant the special lookup for compressed files.

Some unrelated CLI tests don't work for me both on master and on this PR.

@thornjad is there something else I should do for this PR to be approved?
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grybykm grybykm grybykm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fails to serve Unity/Brotli Missing HTTP Response Header "Content-Encoding: gzip"
2 participants
@De-Panther @grybykm