Skip to content
This repository has been archived by the owner on Feb 8, 2024. It is now read-only.

AjaxModelSelect OPTIONS header issue #379

Closed
grzegorzbialy opened this issue Dec 18, 2017 · 11 comments
Closed

AjaxModelSelect OPTIONS header issue #379

grzegorzbialy opened this issue Dec 18, 2017 · 11 comments

Comments

@grzegorzbialy
Copy link

grzegorzbialy commented Dec 18, 2017

OPTIONS header is used for CORS and cannot be trusted for data transfer in request body.

Eg. on AWS lambda request body is cleared with OPTIONS header. I've managed to override this by reading body from wsgi:
request_body = request.environ['wsgi.input']

Sadly, this is not the solution. Microsoft Edge 16 doesn't send request body with OPTIONS request. So AJAX widget is not usable on Edge at all.
After all component should be using POST or GET for this. I suggest using POST with some kind of parameter and provide backward compatibility for OPTIONS. In JS with autocompleter initialization provide parametrization:

var autocompleteType = <setting>;  // <-- this should be OPTIONS or POST. OPTIONS for backward compatibility.
var url = '.';
if (autocompleteType === "POST") {
   url = '.?autocomplete';
}
(...)
.devbridgeAutocomplete({
  (...)
  serviceUrl: url,
  showNoSuggestionNotice: true,
  type: autocompleteType
})

That way we could use POST method for both POSTs and autocompletes using URL param.

@kmmbvnr
Copy link
Collaborator

kmmbvnr commented Dec 18, 2017

I don't understand where cross-origin-request happens.

@grzegorzbialy
Copy link
Author

The problem is with AJAX auto completer which queries view using OPTIONS request. It sends data (field, query) in request body and this is wrong. Not all servers (eg. AWS) and browsers (eg. Edge) support this.

@kmmbvnr
Copy link
Collaborator

kmmbvnr commented Dec 18, 2017

Got it.

@kmmbvnr
Copy link
Collaborator

kmmbvnr commented Dec 18, 2017

I think i would switch to the custom header for HTTP OPTIONS request

@grzegorzbialy
Copy link
Author

It also can be tricky - for example, you cannot send Authorization header with OPTIONS request.

@kmmbvnr
Copy link
Collaborator

kmmbvnr commented Dec 18, 2017

Who would ever use Authorization with django?

@grzegorzbialy
Copy link
Author

This bug is not a problem with Django but with browsers. I was using Authorization header as an example of setting custom header for OPTIONS request - browser can drop it. OPTIONS is not ment to be use this way and that's the whole problem. Microsoft Edge case proves that there will be a problem to test it with all major browsers. 🙂

@kmmbvnr
Copy link
Collaborator

kmmbvnr commented Dec 18, 2017

fast googling shows that custom headers used with options requests, and no one blame

@grzegorzbialy
Copy link
Author

Ok, I've made a simple test:

<script>
    $(function() {
        $.ajax({
            url: '.',
            method: 'OPTIONS',
            beforeSend: function(xhr){xhr.setRequestHeader('X-Test-Header', 'test-value');}
        })
    });
    </script>

Looks like it's working on Chrome, IE11 and Edge 16. I don't know how about AWS which clears request body on server-side.

Why you do not want to make request method customizable?

@kmmbvnr
Copy link
Collaborator

kmmbvnr commented Dec 18, 2017

B/c if would clash with existing form processing code. I think using custom header for same-origin-request is pretty safe. HTTP is all about custom headers.

@kmmbvnr
Copy link
Collaborator

kmmbvnr commented Dec 19, 2017

Fixed in https://github.com/kmmbvnr/material-pro/commit/a8e1f5f404aec64fe40f6fb4a0c2c40e5414aa61

django-material-pro==1.1.3 released

@kmmbvnr kmmbvnr closed this as completed Dec 19, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants