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

Sandbox - LINK is sent via POST with _method parameter (UNLINK is not) #621

Closed
JakubFojtik opened this issue May 9, 2015 · 8 comments · Fixed by #694
Closed

Sandbox - LINK is sent via POST with _method parameter (UNLINK is not) #621

JakubFojtik opened this issue May 9, 2015 · 8 comments · Fixed by #694

Comments

@JakubFojtik
Copy link

Sending a LINK request is done via method faking, so it sends a POST.
I believe it is a bug, since UNLINK and other methods are used directly.
Also, since LINKing is used on a non-collection resource like /something/1, the POST method would be disabled on it and a '405 Method Not Allowed' error is thrown.

If it is because of the firefox bug, that one seems to be fixed already.

@willdurand
Copy link
Collaborator

Yes, that's because of Firefox's bug. Should we change this behavior now? Willing to work on it?

@tonivdv
Copy link
Contributor

tonivdv commented Aug 10, 2015

Hello @willdurand ,

I've got this issue with Google Chrome as well ...

The lines causing the issue is in the file "layout.html.twig" starting at line 524

// Workaround for Firefox bug and a thereby resulting nginx incompatibility
if (method == "LINK") {
  method = "POST";
  params._method = "LINK";
}

Fix from 4fa50eb

Reverting this works on Chrome. If it's a firefox workaround, maybe it should only be executed when it's a firefox browser?

Let me know your thoughts, and I can PR if needed.

Cheers

@willdurand
Copy link
Collaborator

Let's apply this fix only on Firefox then :)

@tonivdv
Copy link
Contributor

tonivdv commented Aug 11, 2015

@willdurand Will do ;)

@tonivdv
Copy link
Contributor

tonivdv commented Aug 11, 2015

@willdurand I just checked with firefox ... it fails too ... so I guess we can simply remove it? /cc @JakubFojtik

@willdurand
Copy link
Collaborator

@tonivdv ok then

@foaly-nr1
Copy link

Any progress on this? 👍

@tonivdv
Copy link
Contributor

tonivdv commented Oct 12, 2015

@foaly-nr1 Well I PR a change, but it made test fail however I don't get why they failed.

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

Successfully merging a pull request may close this issue.

4 participants