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

Set up a Warning header for minification failure #923

Merged
merged 1 commit into from
Mar 9, 2016

Conversation

Martii
Copy link
Member

@Martii Martii commented Mar 9, 2016

  • New dep rfc2047 to allow external viewing of UglifyJS2 message warning via header... this one appears to follow the spec as closely as possible unlike some others
  • No known guarantee that messages from UglifyJS2 will be ISO-8859-1 so Q encode under RFC2047
  • Since it is MIME format typically this should be wrapped at 76 characters... however express/node throws an exception... current deviation from the spec
  • express currently only handles one Warning header which isn't to spec as well... see http://expressjs.com/en/api.html#res.set
  • node currently only handles one Warning header which isn't spec as well... see https://nodejs.org/api/http.html#http_response_setheader_name_value
  • express should be sending HTTP/1.1 so no matching date should be required for compliance as per spec

Ref:

Applies to #432


Additional refs:

Adding needs mitigation label for long-term down-the-line revisiting

* New dep *rfc2047* to allow external viewing of *UglifyJS2* message warning via header... this one appears to follow the spec as closely as possible unlike some others
* No known guarantee that `message`s from *UglifyJS2* will be ISO-8859-1 so Q encode under RFC2047
* Since it is MIME format typically this should be wrapped at 76 characters... however *express*/*node* throws an exception... current deviation from the spec
* *express* currently only handles one `Warning` header which isn't to spec as well... see http://expressjs.com/en/api.html#res.set
* *node* currently only handles one `Warning` header which isn't spec as well... see https://nodejs.org/api/http.html#http_response_setheader_name_value
* *express* should be sending HTTP/1.1 so no matching date should be required for compliance as per spec

Ref:
* http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.46

Applies to OpenUserJS#432
@Martii Martii added enhancement Something we do have implemented already but needs improvement upon to the best of knowledge. CODE Some other Code related issue and it should clearly describe what it is affecting in a comment. labels Mar 9, 2016
Martii added a commit that referenced this pull request Mar 9, 2016
Set up a `Warning` header for minification failure

Auto-merge
@Martii Martii merged commit fa6fe71 into OpenUserJS:master Mar 9, 2016
@Martii Martii deleted the Issue-432warningHeader branch March 9, 2016 22:40
@Martii Martii added the needs mitigation Needs additional followup. label Mar 9, 2016
Martii pushed a commit to Martii/OpenUserJS.org that referenced this pull request Mar 10, 2016
… present

* Appears that the `«` and the `»` aren't in a *UglifyJS2* code search... so do this on our end so it's more readable

Applies to OpenUserJS#432 and post fix for OpenUserJS#923 ... also suggested at mishoo/UglifyJS#1001 (comment)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CODE Some other Code related issue and it should clearly describe what it is affecting in a comment. enhancement Something we do have implemented already but needs improvement upon to the best of knowledge. needs mitigation Needs additional followup.
Development

Successfully merging this pull request may close these issues.

None yet

1 participant