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

Release 4.15 #3187

Merged
merged 24 commits into from
Mar 1, 2017
Merged

Release 4.15 #3187

merged 24 commits into from
Mar 1, 2017

Conversation

dougwilson
Copy link
Contributor

@dougwilson dougwilson commented Jan 29, 2017

This is a tracking issue for release 4.15.

Please keep feature requests in their own issues

If you want to make a comment on a particular change, please make the comment in the "Files changed" tab so comments are not lost during a rebase.

List of changes for release:

Testing this release

If you want to try out this release, you can install it with the following commands:

$ npm cache clean express
$ npm install expressjs/express#4.15

Owners/collaborators: please do not merge this PR :)

@dougwilson dougwilson self-assigned this Jan 29, 2017
@dougwilson dougwilson force-pushed the 4.15 branch 2 times, most recently from a3708a0 to 44ac899 Compare February 5, 2017 02:36
@dougwilson dougwilson force-pushed the 4.15 branch 2 times, most recently from 0a5919f to 049259e Compare February 16, 2017 06:19
@LinusU
Copy link
Member

LinusU commented Feb 19, 2017

Just raising the question, is it considered non-breaking that the current ETags will be invalidated? This could potentially cause files to be unnecessarily re-downloaded, but maybe that isn't an issue?

@dougwilson
Copy link
Contributor Author

I have the exact same thought, of course. I saw it as a minor, based on (1) we have changed it multiple times throughout 4.x without backlash already (2) we don't actually promise what they look like and (3) semver only really accounts for the API used.

I'm open to more discussion on it :) but as for the reason behind the change is that it is actually a bit faster and will work OOB on FIPS Node.js builds now.

@dougwilson
Copy link
Contributor Author

4.9.0, 4.10.0 and 4.13.0 were all releases where the format of the default ETags were altered in some way, FWIW

@LinusU
Copy link
Member

LinusU commented Feb 19, 2017

Sounds good 👍

Tried google OOB on FIPS but didn't get anything good 😄 mind pointing me in the right direction?

@dougwilson
Copy link
Contributor Author

Sorry, OOB meant "out of the box" and there is a special build of Node.js called FIPS mode that disables things like MD5.

@dougwilson
Copy link
Contributor Author

So just from what's left to do, I'm thinking maybe a good target release frame would be this weekend. After the changes settle down, we can propose a more concrete date, just figured I'd toss out an idea general date.

@dougwilson
Copy link
Contributor Author

I believe that the Release 4.15 is code complete at this point. Please take a look for anything either (a) missing or (b) shouldn't belong. If there are no surfaced issues, I propose a release sometime Feb 28.

@dougwilson
Copy link
Contributor Author

So doing some testing, I noticed that res.redirect was never updated to add the CSP header and the full HTML document like the dependencies have done for redirects and errors. Ideally it would be great if these were all consistent (so a res.redirect() produces the same output as a redirect from express.static or res.sendFile(), for example). I also see that it is misusing res.format() by assuming that it will execute the callbacks sync while we don't document such a promise (and it doesn't even use res.send()!).

i.m.o the lack of consistency with the responses seems easy enough to just push the release out a day or two to get it fixed up. I'm not going to change anything right now, so if anyone wants to PR a change to any one of the items or say "nope" to this, feel free :)

@dougwilson
Copy link
Contributor Author

So thinking more about it, I'm going to move to just not make any changes and shoot for releasing the existing work as 4.15 Feb 28 in order to get it out and we can always follow up later if necessary :)

@dougwilson
Copy link
Contributor Author

So I'll be publishing this within a couple hours, since I've been made aware that Snyk has published a notice for an issue in one of our dependencies, qs. It is important to note that the issue does not affect Express, since we do not use the feature in which the issue lies, but regardless, automated checkers will not understand this, so getting a published update out will satisfy the checking tools.

I am also debating if it would be useful to even bother publishing a 4.14.2 with the patched version and then delaying this release a bit more, but 🤷‍♂️

@notrab
Copy link
Contributor

notrab commented Mar 1, 2017

Ship it and ship a patch if necessary. Life is shorter than software development if we had to plan it perfectly. 🚢

@dougwilson
Copy link
Contributor Author

Yea, looking at the change, I'm inclined to only ship in 4.15.0, which I am preparing as I type to publish. Just need to add some additional wording after publish to the website regarding the security fix, and that users are not affected, but an updated module is available as part of 4.15.0. /cc @expressjs/express-tc

@dougwilson dougwilson merged commit 9722202 into master Mar 1, 2017
@dougwilson dougwilson deleted the 4.15 branch March 1, 2017 22:37
@techsin
Copy link

techsin commented Mar 26, 2018

THANKS

for adding next('router') although undocumented it's very useful

@dougwilson
Copy link
Contributor Author

Hi @techsin it is documented in http://expressjs.com/en/guide/using-middleware.html with the rest of the flow. If you think it should be documented else where or improvements, don't hesitate to make a pull request 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants