Skip to content

Loading…

weakening http method constraint to allow PUT or POST for uploads. #24

Open
wants to merge 1 commit into from

6 participants

@talg

I needed to support upload acceleration for an existing system that used PUT to send files. weakening the upload method check seemed like the cleanest way to go. notice a few other folks have requested this feature as well.

@mvrhov

any chances in seeing this one merged?

@vkholodkov
Owner

Could you make it configurable with PUTs off by default?

@talg

I'm open to that.

Can you explain your thoughts around why this is would be the way to go vs. just allowing PUT or POST?

@vkholodkov
Owner

Because it does not comply with standarts.

@talg

I can make it configurable if it will make it into the lastest version.

I notice that what is currently in the git repo is way out of date with the latest version... whats the deal?

@edevil

Has it been merged?

@d
d commented

@vkholodkov which part of http standards isn't PUT request compliant with?

@vkholodkov
Owner

You phrase it incorrectly. PUT request is compliant with HTTP standard, but simply changing POST to PUT makes the module violate the definition of PUT method: "The PUT method requests that the enclosed entity be stored under the supplied Request-URI." In other words, if a PUT request is made for certain Request-URI, then the client expects this resource be available under specified Request-URI, while for POST it doesn't. Request-URI in POST request is just a name of an entry point.

@vkholodkov
Owner

Sorry for being so uncooperative -- lot of things going on

@wbond

@vkholodkov So in terms of PUT requiring that a resource be available under a URL, there is no reason a GET request to the same URL would not return the file, right?

I'm having a hard time understanding why supporting PUT and PATCH (without a configuration) would not be a logical choice? I mean someone could choose to implement them in a non-pure-HTTP way, but is that really what the upload module should be worried about? Isn't that the job of the developer and their nginx config?

@vkholodkov
Owner

@wbond PUT is implemented in the way it is supposed to be implemented in DAV module. Of course, I can reimplement PUT in upload module and grab people's attention:) But it doesn't seem to be worth it...

It's not that GET to the same URL would not return the same file, but for PUT Request-URI identifies the resource being upload and for POST it is just an entry point.

@wbond

Hmm, so perhaps you are only considering the situation where the upload module is handling just a file upload?

For instance, with our usage, we have a REST API accepting a "File" record that contains many fields, one of which is an actual file upload. So when the user wants to update some of the fields in this "File" record they can PATCH if they want to change just a few specific fields.

So unless I am misunderstanding something, in our situation, PUT and PATCH support makes sense. The Request-URI we are PUTing to is for a resource that is more than just the file contents.

Does this make sense?

@vkholodkov
Owner

I must admit, I don't understand you. This could be because you have some unusual setup or because of some other reason. Provide more context if you want your questions answered.

@wbond

Ok, so we have a database table that looks like this:

create table File (
    id   INT,
    url  VARCHAR,
    mime_type VARCHAR,
    size INT,
    description VARCHAR
)

Then we have a JSON REST API that allows interacting with it, with URLs such as:

POST /files
GET /files/1
PUT /files/1
PATCH /files/1
DELETE /files/1

Now, when a new File record is created, we use POST to send the file contents and the nginx upload module works awesome (thank you!).

However, when we want to update the File record with new file contents, we want to do it via PUT or PATCH. However, we can't use the upload module since it restricts functionality to just POST.

Let me know if anything is still unclear.

@wbond

@vkholodkov Does this make sense now?

@vkholodkov
Owner

It does. However, upload module simply does not implement PUT and PATCH. That's it.

But you can extend it if you want.

@wbond

No worries.

I have started maintaining a fork of the nginx-upload-upload module that allows it to function in an environment where GET, OPTIONS, PUT and PATCH methods are all used. For anyone interested, they can follow along at https://github.com/veracross/nginx-upload-module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 19, 2011
  1. allow use of PUT or POST method for uploads.

    Tal Garfinkel committed
Showing with 1 addition and 1 deletion.
  1. +1 −1 ngx_http_upload_module.c
View
2 ngx_http_upload_module.c
@@ -438,7 +438,7 @@ ngx_http_upload_handler(ngx_http_request_t *r)
ulcf = ngx_http_get_module_loc_conf(r, ngx_http_upload_module);
- if (!(r->method & NGX_HTTP_POST))
+ if (!(r->method & (NGX_HTTP_POST | NGX_HTTP_PUT)))
return NGX_DECLINED;
u = ngx_http_get_module_ctx(r, ngx_http_upload_module);
Something went wrong with that request. Please try again.