Yelp / bravado

Bravado is a python client library for Swagger 2.0 services
Other
605 stars 117 forks source link

multipart/form-data content type failures #299

Open openfly opened 7 years ago

openfly commented 7 years ago

can't seem to convince the methods to adhere to content-type multipart/form-data

spec for endpoint looks roughly like this:

  '/v4/stream/{sid}/message/create':
    post:
      summary: Post a message to one existing stream.
      description: |
        Post a new message to the given stream. The stream can be a chatroom,
        an IM or a multiparty IM.

        You may include an attachment on the message.

        The message can be provided as MessageMLV2 or PresentationML. Both formats support Freemarker templates.

        The optional parameter "data" can be used to provide a JSON payload containing entity data.
        If the message contains explicit references to entity data (in "data-entity-id" element attributes),
        this parameter is required.

        If the message is in MessageML and fails schema validation
        a client error results

        If the message is sent then 200 is returned.

        Regarding authentication, you must either use the sessionToken which was created for delegated app access
        or both the sessionToken and keyManagerToken together.
      consumes:
        - multipart/form-data
      produces:
        - application/json
      parameters:
        - name: sid
          description: Stream ID
          in: path
          required: true
          type: string
        - name: sessionToken
          description: Authorization token used to make delegated calls.
          in: header
          required: true
          type: string
        - name: keyManagerToken
          description: Key Manager authentication token.
          in: header
          type: string
          required: false
        - name: message
          description: The message payload in MessageML.
          in: formData
          type: string
          required: true
        - name: data
          description: Optional message data in EntityJSON.
          in: formData
          type: string
          required: false
        - name: version
          description: |
            Optional message version in the format "major.minor". If empty, defaults to the latest supported version.
          in: formData
          type: string
          required: false
        - name: attachment
          description: Optional file attachment.
          in: formData
          type: file
          required: false
      tags:
        - Messages
      responses:
        '200':
          description: Message sent.
          schema:
            $ref: '#/definitions/V4Message'
        '400':
          description: 'Client error, see response body for further details.'
          schema:
            $ref: '#/definitions/Error'
        '401':
          description: 'Unauthorized: Session tokens invalid.'
          schema:
            $ref: '#/definitions/Error'
        '403':
          description: 'Forbidden: Caller lacks necessary entitlement.'
          schema:
            $ref: '#/definitions/Error'
        '500':
          description: 'Server error, see response body for further details.'
          schema:
            $ref: '#/definitions/Error'

the example code looks roughly like:

import symphony

from bravado.requests_client import RequestsClient
from bravado.client import SwaggerClient
from bravado.swagger_model import load_file

def main():
    ''' main program loop '''
    conn = symphony.Config('bot.cfg')
    # connect to pod
    try:
        agent, pod, symphony_sid = conn.connect()
        print 'connected: %s' % symphony_sid
    except:
        print 'failed to connect!'
    try:
        http_client = RequestsClient()
        Gagent = SwaggerClient.from_spec(load_file('agent-api-public.yaml'))
        Gagent.swagger_spec.api_url = "https://URIHERE/agent/"
    except Exception as err:
        print err

   try:
        print 'executing codegen method'
        status_code, retstring = agent.send_message(symphony_sid, msgFormat, message)
        # ['__keymngr__', '__session__', '__url__', '__rest__', '__crt__', '__key__']
        stuff = agent.__dict__.values()
        sessionToken = stuff[1]
        keyManagerToken = stuff[0]
        url = 'https://corporate.symphony.com/'
        print "sessionToken : %s" % sessionToken
        print "keyManagerToekn : %s" % keyManagerToken
        print "symphony_sid : %s" % symphony_sid
        message = "<messageML> hello world. </messageML>"
        result = Gagent.Messages.post_v4_stream_sid_message_create(sessionToken=sessionToken, keyManagerToken=keyManagerToken, sid=symphony_sid, message=message).result()
        print result
        print "%s: %s" % (status_code, retstring)
    except Exception as err:
        print err

if __name__ == "__main__":
      main()

execution looks kinda like this ( with some debug prints ):

connected: HDjOWZyd0Qhj_QYOWTigtn___qhFL6j3dA
executing codegen method
sessionToken : REDACTED
keyManagerToken : REDACTED
symphony_sid : REDACTED
[]
False
{'keyManagerToken': 'REDACTED', 'sessionToken': 'REDACTED'}
# These are arrays of dict keys from the request structure that is being passed to the requests session inside of bravado
['body', 'url', 'hooks', 'headers', '_body_position', '_cookies', 'method']
# These are arrays of dict values from the request structure that is being passed to requests session inside of bravado 
['message=%3CmessageML%3E+hello+world.+%3C%2FmessageML%3E',
 'https://redacted/agent/v4/stream/redacted/message/create', {'response': []}, 
{'Content-Length': '55', 
'Accept-Encoding': 'gzip, deflate', 
'keyManagerToken': 'REDACTED', 
'sessionToken': 'REDACTED', 
'Connection': 'keep-alive', 'Accept': '*/*', 
'User-Agent': 'python-requests/2.18.1', 
'Content-Type': 'application/x-www-form-urlencoded'}, 
None, <RequestsCookieJar[]>, 'POST']

# all this says is the response is a 415... the content-type is wrong.
# 'Content-Type': 'application/x-www-form-urlencoded' != multipart/form-data
<Response [415]>
... ( removed cause waste of space )
openfly commented 7 years ago

It's basically choosing the wrong content-type and passing data in the wrong format.

macisamuele commented 7 years ago

@openfly it is possible to capture the real HTTP request sent? It's hard to understand why your Java service returned an HTTP/415.

As a side note could you add in the execution trace that you posted what each line represents?

openfly commented 7 years ago

I cleaned it up a little but the key bits are that the content-type is wrong. it changed to 'Content-Type': 'application/x-www-form-urlencoded'

which triggers the 415. If i manually go into _request_options and change the header it fixes that 415... but still gets a 400 from how it handles the data it's passing as formData. It's not being passed to requests correctly. it shouldn't be being urlencoded ( I believe ).

sjaensch commented 7 years ago

You're not setting the (optional) attachment parameter, which is of type file. Currently we don't explicitly set the content type, but let the HTTP client take care of it in case of file uploads. Check out the relevant source code in bravado-core.

application/x-www-form-urlencoded is the more efficient encoding unless you need to send non-text data; I'm wondering why your Java server is so strict about it. Anyway, it does seem like we're not fully respecting the spec in this case. This would need to be fixed both in the Fido and the requests client.

openfly commented 7 years ago

Yeah that's a bug man.

multi-part form is handled differently than x-www-form-urlencoded for stuff other than file uploads. so if the api expects multi-part form and you hand it x-www-form-urlencoded you will see failures to parse.

basically bravado is failing to set the method appropriately based on the swagger spec.

I can't really speak for the actions of the java server... beyond it doing what it's doing. =/

openfly commented 7 years ago

I'll take a look and see if I have the up front knowledge to fix it without breaking more than I fix.

openfly commented 6 years ago

So this is more a bug with requests than bravado.
https://github.com/requests/requests/issues/1081 <-- requests maintainer basically refuses to support multipart format correctly because he doesn't like how it changes the python module. https://franklingu.github.io/programming/2017/10/30/post-multipart-form-data-using-requests/ <-- may be a work around... ie setting a None for the file attachment, when none is presented.

openfly commented 6 years ago

http://toolbelt.readthedocs.io/en/latest/uploading-data.html#streaming-multipart-data-encoder the toolbelt has a much more robust solution to the issue.

openfly commented 6 years ago

Been a while but ended up revisiting my work around for this in my code. And I had more info on the issue to add here.