David Baron :dbaron: Reporter
	Description • 6 years ago

There's a regression in nightlies that uploading a tileset to mapbox fails, with the error message:
  The request signature we calculated does not match the signature you
  provided.  Check your key and signing method.

Steps to reproduce:
 1. create an account at https://www.mapbox.com/ and log in
 2. go to https://www.mapbox.com/studio/tilesets/
 3. press the "New tileset" button
 4. choose a TIFF image file from the local disk (actually, it probably doesn't even need to be a TIFF image... but it's possible the file needs to be large... I need to investigate the exact conditions here further)
 5. press the "Confirm" button

Actual results:
 1. the Notifications popup appears in the lower right of the window
 2. It contains the error, in red:
      The request signature we calculated does not match the signature you
      provided.  Check your key and signing method.

Expected results:
 1. (as in actual results)
 2. It contains a green progress bar showing progress of the upload.

The from-nightlies regression range is:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9812141ec78239560283181ce610249d181b74b6&tochange=5347c7e4811a1a4d80bcee4119cead9192fdff69
but I'm still running mozregression which has moved on to using taskcluster builds, so I'll have a narrower range in a bit.

	David Baron :dbaron: Reporter
	Comment 1 • 6 years ago

[Tracking Requested - why for this release]:  A regression in behavior of a relatively prominent website (although most users are just seeing their embedded maps rather than creating new ones); this will, however, affect web developers using mapbox to set up maps this way.  There may also be other sites affected by the same underlying problem.

	David Baron :dbaron: Reporter
	Comment 2 • 6 years ago

The window got small enough that I can skim the changesets and see that it's probably bug 1454325, but I'm going to let mozregression finish...

	David Baron :dbaron: Reporter
	Comment 3 • 6 years ago

282:05.35 INFO: Narrowed inbound regression window from [8778c2cf, ca06d80a] (4 builds) to [a0179bdd, ca06d80a] (2 builds) (~1 steps left)
282:05.35 INFO: No more inbound revisions, bisection finished.
282:05.35 INFO: Last good revision: a0179bdde4960184f66612eee83a2fd96238fc1d
282:05.36 INFO: First bad revision: ca06d80acf9b03396f01d37f1cd23fa4a3bdc715
282:05.36 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a0179bdde4960184f66612eee83a2fd96238fc1d&tochange=ca06d80acf9b03396f01d37f1cd23fa4a3bdc715

	Thomas Wisniewski [:twisniewski] Assignee
	Comment 4 • 6 years ago

I was unable to reproduce this just now. I tried making an account and uploading marbles.tif from the site https://www.fileformat.info/format/tiff/sample/index.htm. In both Chrome and Firefox I see an obvious PUT XHR being sent with image/tiff content-type, and what appears to be the same binary data. Then the "new tileset" popup vanishes, and I see no new tileset was actually created, and no error or other messages appearing. The other XHRs I see also appear similar, and I don't get any other indications in the UI that something is wrong (beyond no new tileset being created in either browser).

Perhaps marbles.tif just isn't valid for use as a tileset, and I'll need to try with another sample file? Would I be able to get a copy of a file that's failing for you, dbaron?

	Thomas Wisniewski [:twisniewski] Assignee
	Comment 5 • 6 years ago

(Note that I tried the latest Firefox nightly on Linux and Chrome 68).

	David Baron :dbaron: Reporter
	Comment 6 • 6 years ago

So I haven't ever uploaded a tileset that's *less* than a few hundred megabytes... so I'd either:
 (a) need to figure out how to transfer you the huge file, or
 (b) figure out how to use the gdal tools to subset my file.

	David Baron :dbaron: Reporter
	Comment 7 • 6 years ago

It may well require the very large file, since in the working build, it seems to be splitting it across a large number of PUT and OPTIONS requests.  You might just try taking a 350MB file and renaming it to something.tif .

	David Baron :dbaron: Reporter
	Comment 8 • 6 years ago

(I don't think it should require any particular contents, since the failure happens at the start of the file upload, so I don't think it would have looked at the file contents yet.)

	Thomas Wisniewski [:twisniewski] Assignee
	Comment 9 • 6 years ago

Alright, thanks for the tips. I'll try doing that tomorrow (it's quite late here).

However if you feel this warrants a quick back-out while I'm away, the new XHR behaviour is gated behind dom.xhr.standard_content_type_normalization (which bug 1454325 added to StaticPrefList.h).

	Thomas Wisniewski [:twisniewski] Assignee
	Comment 10 • 6 years ago

Yes, I see the error now. It doesn't even try to PUT when the file is large enough. It stops during the initial POST, which returns a 403 failure with the following response document:

><?xml version="1.0" encoding="UTF-8"?>
><Error><Code>SignatureDoesNotMatch</Code><Message>The request signature we calculated does not match the signature you provided. Check your key and signing method.</Message><AWSAccessKeyId>ASIATNLVGLR2F6DLSAFE</AWSAccessKeyId><StringToSign>POST
> 
>image/tiff;charset=UTF-8
> 
>x-amz-date:Thu, 27 Sep 2018 15:11:27 GMT
>x-amz-security-token:FQoGZXIvYXdzEHkaDJUQXXdN+gvXmY5E7iLOAlQBteyFbJGRCv6TdbN4Dm6/ZH8BH2ZZ6xBziZ9eO5YHE7+nPb+uQyHG//MHpq5AiYn7FKwFE3tk7Cskz7F6Ko6olHFZB4XLKHDKf2iPaWHYqYMru4RKs7ax9r3mGwZxc15n9hm05z4/gTCfqx9mue1LDRG5B86r3hcAWCOoCVvxSdlMEQAB725Ba/efwqCYEGYTXRNz/i0nMWURkwB+lDYsNjZexPP9msYf1uy/TaT7HH1XkKAxe4lzDcEDbA+dyp32s6C0QoDtFz6M/I4xTutmDwixNGwmUkLOmk6iec1O6yPHFlCUM74+ln45kkeo43QAqERgUGd0lqbZ3sEOWaT91wrLv08OvehDcg1DpOrFUUJBCdCRhLdY87MOPNbfsiouJqs7jO1cVTDC3pTtOCgR9CW8bAj4RaBK1jH2m5HiJ3j8JMjI2OLbMx5v9Nkov+az3QU=
>x-amz-user-agent:aws-sdk-js/2.1.46
>/tilestream-tilesets-production/7e/_pending/i5tn6i61app2q7w0i2nvpkmjc/webcompattester?uploads</StringToSign><SignatureProvided>8rSzNSltXgR47WhYIt4F1xRsd/4=</SignatureProvided><StringToSignBytes>50 4f 53 54 0a 0a 69 6d 61 67 65 2f 74 69 66 66 3b 63 68 61 72 73 65 74 3d 55 54 46 2d 38 0a 0a 78 2d 61 6d 7a 2d 64 61 74 65 3a 54 68 75 2c 20 32 37 20 53 65 70 20 32 30 31 38 20 31 35 3a 31 31 3a 32 37 20 47 4d 54 0a 78 2d 61 6d 7a 2d 73 65 63 75 72 69 74 79 2d 74 6f 6b 65 6e 3a 46 51 6f 47 5a 58 49 76 59 58 64 7a 45 48 6b 61 44 4a 55 51 58 58 64 4e 2b 67 76 58 6d 59 35 45 37 69 4c 4f 41 6c 51 42 74 65 79 46 62 4a 47 52 43 76 36 54 64 62 4e 34 44 6d 36 2f 5a 48 38 42 48 32 5a 5a 36 78 42 7a 69 5a 39 65 4f 35 59 48 45 37 2b 6e 50 62 2b 75 51 79 48 47 2f 2f 4d 48 70 71 35 41 69 59 6e 37 46 4b 77 46 45 33 74 6b 37 43 73 6b 7a 37 46 36 4b 6f 36 6f 6c 48 46 5a 42 34 58 4c 4b 48 44 4b 66 32 69 50 61 57 48 59 71 59 4d 72 75 34 52 4b 73 37 61 78 39 72 33 6d 47 77 5a 78 63 31 35 6e 39 68 6d 30 35 7a 34 2f 67 54 43 66 71 78 39 6d 75 65 31 4c 44 52 47 35 42 38 36 72 33 68 63 41 57 43 4f 6f 43 56 76 78 53 64 6c 4d 45 51 41 42 37 32 35 42 61 2f 65 66 77 71 43 59 45 47 59 54 58 52 4e 7a 2f 69 30 6e 4d 57 55 52 6b 77 42 2b 6c 44 59 73 4e 6a 5a 65 78 50 50 39 6d 73 59 66 31 75 79 2f 54 61 54 37 48 48 31 58 6b 4b 41 78 65 34 6c 7a 44 63 45 44 62 41 2b 64 79 70 33 32 73 36 43 30 51 6f 44 74 46 7a 36 4d 2f 49 34 78 54 75 74 6d 44 77 69 78 4e 47 77 6d 55 6b 4c 4f 6d 6b 36 69 65 63 31 4f 36 79 50 48 46 6c 43 55 4d 37 34 2b 6c 6e 34 35 6b 6b 65 6f 34 33 51 41 71 45 52 67 55 47 64 30 6c 71 62 5a 33 73 45 4f 57 61 54 39 31 77 72 4c 76 30 38 4f 76 65 68 44 63 67 31 44 70 4f 72 46 55 55 4a 42 43 64 43 52 68 4c 64 59 38 37 4d 4f 50 4e 62 66 73 69 6f 75 4a 71 73 37 6a 4f 31 63 56 54 44 43 33 70 54 74 4f 43 67 52 39 43 57 38 62 41 6a 34 52 61 42 4b 31 6a 48 32 6d 35 48 69 4a 33 6a 38 4a 4d 6a 49 32 4f 4c 62 4d 78 35 76 39 4e 6b 6f 76 2b 61 7a 33 51 55 3d 0a 78 2d 61 6d 7a 2d 75 73 65 72 2d 61 67 65 6e 74 3a 61 77 73 2d 73 64 6b 2d 6a 73 2f 32 2e 31 2e 34 36 0a 2f 74 69 6c 65 73 74 72 65 61 6d 2d 74 69 6c 65 73 65 74 73 2d 70 72 6f 64 75 63 74 69 6f 6e 2f 37 65 2f 5f 70 65 6e 64 69 6e 67 2f 69 35 74 6e 36 69 36 31 61 70 70 32 71 37 77 30 69 32 6e 76 70 6b 6d 6a 63 2f 77 65 62 63 6f 6d 70 61 74 74 65 73 74 65 72 3f 75 70 6c 6f 61 64 73</StringToSignBytes><RequestId>C1199F7E96904A83</RequestId><HostId>Z9W0nGllRjFeNUESDg3Ee+baweP5l8+yJeqMxpDWidux1Pov5+QnDRSJjiYnT+S4hBvJbRl3+IA=</HostId></Error>

It turns out that they are overriding the MIME type to: image/tiff; charset=UTF-8
But per the current XHR spec, this is normalized to: image/tiff;charset=UTF-8
Note the missing space.

Apparently this is enough to break their "signature checker" on Mapbox.

Anne, you've been sorting all of this MIME stuff out. Is this worrying enough to change the spec, or should we reach out to Mapbox? (Have other vendors already committed to matching the current spec?)

For the sake of completeness, the code that does this is here: https://searchfox.org/mozilla-central/source/dom/xhr/XMLHttpRequestMainThread.cpp#2864
With the actual spaceless serialization done here: https://searchfox.org/mozilla-central/source/dom/base/MimeType.cpp#232
And the spec for that serialization is here: https://mimesniff.spec.whatwg.org/#serializing-a-mime-type

	Anne (:annevk)
	Comment 11 • 6 years ago

Okay, so this is about what happens if you set Content-Type yourself, parsing it doesn't fail, and it contains a charset parameter (currently part of step 4.4 in send()'s definition). There were differences across browsers here, but nobody has a proper parse/serialize step I think and per https://github.com/whatwg/xhr/pull/176#issuecomment-381544021 nobody is updated yet apart from Firefox. (There were differences across browsers though.)

I'm not sure how to judge the impact of this. If this is a common technique I guess we have to revert and figure out a new way to do things. If this is just this site I'd much prefer getting this site fixed as other techniques for manipulating MIME types would be far less sound.

	Thomas Wisniewski [:twisniewski] Assignee
	Comment 12 • 6 years ago

dbaron, do you have any thoughts on this given Anne's feedback?

	Josh Matthews [:jdm]
	Comment 13 • 6 years ago

I've encountered another site regression from bug 1454325 in bug 1496754, if that changes the equation.

	Julien Cristau [:jcristau]
	Comment 14 • 6 years ago

The regressing change was disabled in bug 1499136.

	David Baron :dbaron: Reporter
	Comment 15 • 6 years ago

I guess it seems like the spec needs to be more compatible with existing web content here, unless you have a good reason why it's worth spending the somewhat-substantial amount of work needed to do otherwise.

	Anne (:annevk)
	Comment 16 • 6 years ago

Yeah, it was changed per bug 1454325 comment 26. I didn't realize there were still open needinfos around this, sorry.