From 76bcc30af73b4de3df4b36a1362efb501800beaa Mon Sep 17 00:00:00 2001 From: Bernd Sitzmann Date: Wed, 16 Jan 2019 22:50:12 -0700 Subject: [PATCH] Fix version issue when actual patch version >= 10 Today we encountered an incident where after an MCS deploy, which changed the summary version from 1.3.9. to 1.3.10, caused RB to rerender the summary for every page.[1] The problem is that the comparison previously used failed when the patch version number was 10 or greater and the expected patch version number is < 10. In `node`: > "1.3.9" > "1.3.7" true > "1.3.10" > "1.3.7" false I then considered using parseInt() but since we're using version numbers with 2 dots that fails, too: > parseInt("1.3.10") > parseInt("1.3.7") false So, I propose using the semver module which should handle that properly. Unfortunately the current structure of this module doesn't make it easy to create a unit test without some refactoring. [1] https://wikitech.wikimedia.org/wiki/Incident_documentation/20190117-Parsoid --- lib/ensure_content_type.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/ensure_content_type.js b/lib/ensure_content_type.js index 9d19879c1..cee8a8b2f 100644 --- a/lib/ensure_content_type.js +++ b/lib/ensure_content_type.js @@ -3,6 +3,7 @@ const cType = require('content-type'); const P = require('bluebird'); const mwUtil = require('./mwUtil'); +const semver = require('semver'); // Utility function to split path & version suffix from a profile parameter. function splitProfile(profile) { @@ -51,7 +52,7 @@ function checkContentType(hyper, req, next, expectedContentType, responsePromise const actualProfileParts = splitProfile(actualProfile); const expectedProfileParts = splitProfile(expectedProfile); if (actualProfileParts.path === expectedProfileParts.path - && actualProfileParts.version > expectedProfileParts.version) { + && semver.gt(actualProfileParts.version, expectedProfileParts.version)) { return res; } }