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

Calling an endpoint which returns file does not download content #374

Closed
sean-heller opened this issue Jan 17, 2014 · 16 comments
Closed

Calling an endpoint which returns file does not download content #374

sean-heller opened this issue Jan 17, 2014 · 16 comments
Milestone

Comments

@sean-heller
Copy link

When calling an endpoint that has a response with Content-Disposition attachment or a Content-Type such as application/zip, swagger will only show the bytes in the response, instead of a download prompt from the browser.

@jlg7
Copy link

jlg7 commented Jul 11, 2014

+1 for this fella. definitely would help keep users in swagger ui for the entire API

@sdumetz
Copy link

sdumetz commented Jul 17, 2014

👍 A workaround that could be OK for me would be to simply prevent the zip to display as bytes in browser for it causes endless lag even for "small" files.

@bthule
Copy link

bthule commented Oct 28, 2014

+1. I have content type of application/octet-stream, and I was surprised to see all the binary displayed in the web page instead of being prompted for a download.

@brokensandals
Copy link

(This issue appears to be the same as #600)

I made an attempt to implement a 'save as file' link in the response section, which would be one way to address this. At least for browsers that support the File API, it's pretty easy from a UI perspective: when the request finishes, you create a Blob containing the content of the swagger response, use window.URL.createObjectURL to get a url to the blob, and put that in a link. (This approach still requires that the whole response be loaded into memory; I don't know how to get around that.)

The problem I ran into (IIUC) is that the response contents are already coerced to Unicode by the time they're returned from swagger-js, so non-text responses will be mangled. So this might require first updating swagger-js to support binary response bodies - I think this issue is relevant: swagger-api/swagger-js#123

@fehguy
Copy link
Contributor

fehguy commented Dec 3, 2014

Hi, a couple thoughts.

  • The default swagger-js transport library uses shred. The toString() is deep in that code and is probably too difficult to unwrap
  • There is an option to use jQuery as the transport, which I believe is easier to configure for handling the response.

That said, you should be able to configure swagger-ui with useJquery: true and handle the binary response from jQuery directly.

@tartakynov
Copy link

+1

@brokensandals
Copy link

@fehguy thanks for your advice. I was able to get a hacky implementation working, but there are a couple obstacles to doing a clean implementation in swagger-ui:

  • The only way to do binary downloads in jQuery is to write a custom transport. You more or less have to copy/paste the default transport (about 150 lines) straight out of jQuery and modify it - I couldn't find any way to just inject the extra functionality into the existing transport at runtime. (Someone did wrap a binary transport into a plugin but it doesn't look like it retains all the functionality of the default transport.)
  • When constructing its own response object, swagger-js only uses the responseText field from the jQuery response object. If the custom transport uses some custom field to store the binary response, swagger-js needs to be modified to copy that forward. On the other hand, if the custom transport stores the binary response in the responseText field (doesn't seem like a great idea), swagger-js needs to be modified to not try to call JSON.parse on it since it won't be a string.

So this feature seems to require modifying swagger-js; I see a couple options:

  1. include a custom transport in swagger-js and add a field to the response object to contain the blob response
  2. support some options in swagger-js so that consumers can plug in their own custom transport and get additional fields copied to the response object

@fehguy fehguy modified the milestone: v2.1.0-M2 Jan 28, 2015
@fehguy
Copy link
Contributor

fehguy commented Jan 29, 2015

Hi @brokensandals got it. Did you look at newer versions of JQuery?

I'd love to move this logic all into swagger-js. The shred http client has a pretty low-level functionality to it, and we can probably work in the code somehow. Can you share the changes you made to jQuery in a gist so I can see?

@brokensandals
Copy link

Did you look at newer versions of JQuery?

I've mostly looked at 1.8.0 and 1.9.1 so far, though based on jquery/jquery#1525 it doesn't sound like the situation has really changed.

Can you share the changes you made to jQuery in a gist so I can see?

Here's the custom transport: brokensandals@fab1bfe#diff-4

This gist shows the diff of that with the jQuery file it's based on: https://gist.github.com/brokensandals/be6911a3f7d06f93b15d (the xhrOnUnloadAbort stuff isn't relevant). Basically it just has to set the response type on the XHR, avoid calling xhr.responseText (which would trigger errors), and copy the xhr response into the responses object. See e.g. this blog post.

The rest of the commit above (which is a little noisy since I regenerated the dist files) shows how it can be used: the jQuery ajax options must set dataType to route to the transport and responseFields to ensure the blob field is copied into the jQuery response object.

@tarikbenmerar
Copy link

@brokensandals I have create a plugin that resolves this issue as you said in the comment there won't be support in jQuery any soon. The plugin here : https://github.com/acigna/jquery-ajax-native/. Good luck, hope you find it very useful.

@fehguy
Copy link
Contributor

fehguy commented Feb 3, 2015

Thank you @tarikbenmerar ! Have you ever integrated it with swagger-ui? I'll look into this in the next few days.

@tarikbenmerar
Copy link

@fehguy no, I have came here because someone has referenced the issue jquery/jquery#1525.

@calfzhou
Copy link

calfzhou commented Mar 7, 2015

+1

@mohsen1
Copy link
Contributor

mohsen1 commented Mar 26, 2015

#1079

@mohsen1 mohsen1 closed this as completed Mar 26, 2015
@hamx0r
Copy link

hamx0r commented Apr 22, 2015

Looking at the code in this pull within OperationView.js, it looks like contenType is first tested for specific types, then eventually assumed to be a generic file download (line 606-). However, when running the respective develop-2.0 branch, Chrome's Developer Tools console shows Uncaught TypeError: Cannot read property 'test' of undefined in swagger-ui.js:21394 (same lines of code mentioned in OperationView.js). The progress animation runs forever.

The Content-Disposition in my response is attachment; filename=file.zip and the response is a valid ZIP. Perhaps the filename=file.zip part is messing up the test? The relevant parts of the Swagger Spec feeding the UI are:

        "/widgets/csv/{widget_id}": {
            "parameters": [{
                "required": true,
                "type": "string",
                "name": "widget_id",
                "in": "path"
            }],
            "get": {
                "tags": ["widgets"],
                "summary": "\n        Return CSV of widgets, one for each widget \"id\" (a CSV list of id's is acceptable)\n        ",
                "responses": {
                    "200": {
                        "type": "file",
                        "description": "CSV ZIP File"
                    }, ...
                },
                "parameters": [{
                    "name": "widget_id",
                    "required": true,
                    "in": "path",
                    "type": "string",
                    "description": "CSV list of widget IDs"
                },

The Swagger 2.0 Spec says to use the type of file for files instead of a schema - not sure if my syntax is wrong there because the Swagger Editor says that type is not allowed here.

What's the expected Swagger JSON syntax and Header fields in order to allow file downloads?

@webron
Copy link
Contributor

webron commented Apr 22, 2015

@hamx0r - can't comment in the behavior you experience, but spec-wise, it does say to use type instead of schema but rather your schema can have the "type": "file" where elsewhere it is not permitted.

So response part is not a valid spec. It should be:

                "responses": {
                    "200": {
                        "schema": {
                            "type": "file"
                        },
                        "description": "CSV ZIP File"
                    }, ...
                }

FWIW, it's normally better to open new issues on rather than comment on closed ones if more than a few days have past. It's true that we get email notifications but it's easy to miss a few in the load and we can't follow comments on closed issues otherwise. You can always refer to a closed issue by using the reference number.

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

No branches or pull requests