TritonDataCenter / node-manta

Node.js SDK for Manta
75 stars 54 forks source link

`createUpload` incorrectly handles some target object headers #311

Closed jordanhendricks closed 7 years ago

jordanhendricks commented 7 years ago

While working on the mmpu man page (#joyent/node-manta#302), I wrote up a couple examples of mmpu create, including one that added a content-type and access-control-allow-origin header to the object:

As a part of my review of the man page, I tested out my examples to ensure they worked. To my surprise, the following example failed with a confusing error:

$ mmpu create ~~/stor/foo.txt -c 3 -H 'content-type:text/plain' \
                                                 -H 'access-control-allow-origin:*'
mmpu create: error: text/plain

The error message itself was not very helpful, but seemed to be related to the fact that I specified a content-type header for the object. I ran the command again with -v and saw something interesting:

$ mmpu create ~~/stor/foo.txt -c 3 -H 'content-type:text/plain' \
                                                 -H 'access-control-allow-origin:*' -v 2>&1 | bunyan
[2017-06-13T06:17:59.026Z] DEBUG: mmpu/MantaClient/17103 on joyadmins-MacBook-Pro-4.local (/opt/pkg/lib/node_modules/manta/lib/client.js:3346 in createUpload): createUpload: entered (req_id=968b8bfb-d07e-4501-92ce-cd0288d069cb, path=/jhendricks/uploads)
[2017-06-13T06:17:59.066Z] TRACE: mmpu/MantaClient/17103 on joyadmins-MacBook-Pro-4.local (/opt/pkg/lib/node_modules/manta/node_modules/restify-clients/lib/HttpClient.js:314 in rawRequest): request sent
    POST /jhendricks/uploads HTTP/1.1
    Host: manta.staging.joyent.us
    content-type: text/plain
    access-control-allow-origin: *
    accept: application/json
    x-request-id: 968b8bfb-d07e-4501-92ce-cd0288d069cb
    date: Tue, 13 Jun 2017 06:17:59 GMT
    authorization: Signature keyId="/jhendricks/keys/aa:14:68:f8:18:bb:a0:88:f5:62:35:b6:35:b6:83:29",algorithm="rsa-sha256",headers="date",signature="R/chTMbkMVAQbMnF6MOleOyKi9Zr2LhEQRyvrvSlOBFjwSgjlgDAYDOR/lGOI8pUvKCpuw0k+GiJ6FV435vHcv9B99luwPPDWh7FC+93kpbYZUOnh+LkAmy3EkFI/Hb83IbIyePE3FqBqfQEsxfHIi4xbQr+me9pXeS2e7cF1uh0ZfnHPQhqu4mweY+MiBxxncnayYQPoY9iJM0qYdqyZpjxuwnzc9C9PsGKR8V+hdP1U1gjCfc2/TYWSObzD7wAJ0rRrTQbmVWkqJI5Ez5ue8fQGO4rUKzyKqyQfBZxZ+pXH2hk706rt4cWrDNWpho2XyJeb6f/FqgMVgxJm8aROQ=="
    user-agent: restify/1.4.1 (x64-darwin; v8/3.28.71.19; OpenSSL/1.0.2j) node/0.12.17
    accept-version: ~1.0
    content-length: 136
    content-md5: U7thg1vlOr3u65jZEHqeTg==
[2017-06-13T06:17:59.955Z] TRACE: mmpu/MantaClient/17103 on joyadmins-MacBook-Pro-4.local (/opt/pkg/lib/node_modules/manta/node_modules/restify-clients/lib/HttpClient.js:210 in onResponse): Response received
    HTTP/1.1 415 Unsupported Media Type
    content-type: application/json
    content-length: 59
    content-md5: AkFuEqCXnWrRjsvutVdTDg==
    date: Tue, 13 Jun 2017 06:17:59 GMT
    connection: keep-alive
    x-request-received: 1497334679055
    x-request-processing-time: 900
[2017-06-13T06:17:59.960Z] TRACE: mmpu/MantaClient/17103 on joyadmins-MacBook-Pro-4.local (/opt/pkg/lib/node_modules/manta/node_modules/restify-clients/lib/StringClient.js:193 in done):
    body received:
    {"code":"UnsupportedMediaTypeError","message":"text/plain"}
[2017-06-13T06:17:59.962Z] DEBUG: mmpu/MantaClient/17103 on joyadmins-MacBook-Pro-4.local (/opt/pkg/lib/node_modules/manta/lib/client.js:3364): text/plain (req_id=968b8bfb-d07e-4501-92ce-cd0288d069cb, path=/jhendricks/uploads)
    UnsupportedMediaTypeError: text/plain
        at parseResponse (/opt/pkg/lib/node_modules/manta/node_modules/restify-clients/lib/JsonClient.js:73:26)
        at IncomingMessage.done (/opt/pkg/lib/node_modules/manta/node_modules/restify-clients/lib/StringClient.js:207:13)
        at IncomingMessage.g (events.js:199:16)
        at IncomingMessage.emit (events.js:129:20)
        at _stream_readable.js:908:16
        at process._tickCallback (node.js:355:11)
mmpu create: error: text/plain

Here we see an "UnsupportedMediaTypeError" from the server-side for the media type "text/plain". This indicates that the content type of the request was sent as "text/plain" instead of the actual content type (which is JSON here). This is a clear flag that user-specified header for the target object was being sent as an actual header on the object instead of as data in the JSON request.

The culprit here is that createUpload uses a client helper called createOptions to create the headers for the request, which in turn uses the headers key on options object passed from the CLI script. The headers key here unintentionally has a dual meaning -- both the headers to store on the target MPU object, and the headers for the request.

I think the cleanest fix is to rename the headers key on the options blob that is passed to the Manta client, so that there is a clear separation because request headers and headers that are actually just data on the MPU create request.

jordanhendricks commented 7 years ago

CR at: https://cr.joyent.us/#/c/2079/.

Summary

After some discussion on the CR, I've decided it's best to fix this issue without changing the arguments to the createUpload method on the Manta client, which would constitute a major version bump. Instead, I changed the way the headers options is interpreted by the method to prevent them from being interpreted as headers on the request itself.

I added a test for this bug that creates an upload with headers on the target object that are also headers added to requests via the "opts.headers" argument for most methods on the client.

Testing Notes

First, let's make sure the example from the man page works correctly now:

➜  node-manta git:(master) ✗ bin/mmpu create ~~/stor/foo.txt -c 3 -H 'content-type:text/plain' \
                                                 -H 'access-control-allow-origin:*'
ca9d7c6f-a1be-6d2f-943f-e860b00c616b

Additionally, I ran the node-manta test suite, including the additional test, against my Manta standup, and all tests passed.

This change is make prepush clean.