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.

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

gzip middleware now strips encoding header #2

Merged
merged 3 commits into from
Apr 27, 2015
Merged

gzip middleware now strips encoding header #2

merged 3 commits into from
Apr 27, 2015

Conversation

thomas4019
Copy link

This adds a cleaner way to prevent double encoding. Puts back io.Copy simplified version of writing body which I accidentally replaced earlier.

@mholt
Copy link
Member

mholt commented Apr 27, 2015

Oops, I think I beat you to the commit that fixes gzipping. My bad for not communicating that. But if you resolve the merge conflict with HEAD, I'll be happy to merge in the rest of the change!

@thomas4019
Copy link
Author

No worries, this should now be able to be merged in.

mholt added a commit that referenced this pull request Apr 27, 2015
gzip middleware now strips encoding header
@mholt mholt merged commit c33a49f into caddyserver:master Apr 27, 2015
@weingart weingart mentioned this pull request Apr 9, 2016
@hernandev hernandev mentioned this pull request Aug 7, 2016
blaubaer added a commit to blaubaer/caddy that referenced this pull request Jan 3, 2017
mholt pushed a commit that referenced this pull request Jan 3, 2017
* Fix #2 (Replacement doesn't happen - echocat/caddy-filter#2) bug of caddy-filter

* Fixed gofmt issue.

* Remove comment of reason why we do a reorder
@rstupek rstupek mentioned this pull request Apr 2, 2018
augustoroman added a commit to augustoroman/caddy that referenced this pull request Jun 16, 2018
Two problems are fixed by this code simplification:
1. Snippets defined in one import file are strangely not available in
   another.
2. If an imported file had a directive with an argument "import", then
   the rest of the tokens on the line would be converted to absolute
   filepaths.

An example of caddyserver#2 would be the following directive in an imported file:
    basicauth / import secret

In this case, the password would actually be an absolute path to the
file 'secret' (whether or not it exists) in the directory of the imported
Caddyfile.

The problem was the blind token processing to fix import paths in the
imported tokens without considering the context of the 'import' token.

My first inclination was to just add more context (detect 'import' tokens
at the beginning of lines and check the value tokens against defined
snippets), however I eventually realized that we already do all of this
in the parser, so the code was redundant. Instead we just use the current
token's File property when importing. This works fine with imported tokens
since they already have the absolute path to the imported file!

Fixes caddyserver#2204
augustoroman added a commit to augustoroman/caddy that referenced this pull request Jun 16, 2018
Two problems are fixed by this code simplification:
1. Snippets defined in one import file are strangely not available in
   another.
2. If an imported file had a directive with an argument "import", then
   the rest of the tokens on the line would be converted to absolute
   filepaths.

An example of caddyserver#2 would be the following directive in an imported file:
    basicauth / import secret

In this case, the password would actually be an absolute path to the
file 'secret' (whether or not it exists) in the directory of the imported
Caddyfile.

The problem was the blind token processing to fix import paths in the
imported tokens without considering the context of the 'import' token.

My first inclination was to just add more context (detect 'import' tokens
at the beginning of lines and check the value tokens against defined
snippets), however I eventually realized that we already do all of this
in the parser, so the code was redundant. Instead we just use the current
token's File property when importing. This works fine with imported tokens
since they already have the absolute path to the imported file!

Fixes caddyserver#2204
mholt pushed a commit that referenced this pull request Jun 28, 2018
* Fix a few import problems: snippets and import literals.

Two problems are fixed by this code simplification:
1. Snippets defined in one import file are strangely not available in
   another.
2. If an imported file had a directive with an argument "import", then
   the rest of the tokens on the line would be converted to absolute
   filepaths.

An example of #2 would be the following directive in an imported file:
    basicauth / import secret

In this case, the password would actually be an absolute path to the
file 'secret' (whether or not it exists) in the directory of the imported
Caddyfile.

The problem was the blind token processing to fix import paths in the
imported tokens without considering the context of the 'import' token.

My first inclination was to just add more context (detect 'import' tokens
at the beginning of lines and check the value tokens against defined
snippets), however I eventually realized that we already do all of this
in the parser, so the code was redundant. Instead we just use the current
token's File property when importing. This works fine with imported tokens
since they already have the absolute path to the imported file!

Fixes #2204

* renamed file2 -> fileName

* Fix copy/pasted comment in test.

* Change gzip example to basicauth example.

This makes it more clear how the import side effect is detrimental.
mholt added a commit that referenced this pull request Jul 2, 2019
* Implement encode middleware

* Add missing break; and add missing JSON struct field tag
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants