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 should not be disabled in https whenever a method is not marked as https explictly #348

Merged
merged 1 commit into from
Mar 12, 2014

Conversation

tonivdv
Copy link
Contributor

@tonivdv tonivdv commented Mar 11, 2014

This PR is to enhance issue #283 and fixes issue #330.

What I did is to only disable the sandbox whenever a method has been explicitly marked as https and accessed through http. But whenever you access through https a method not explicitly marked it will allow it too!

@SimonSimCity does that break something for you?

@SimonSimCity
Copy link
Contributor

Have you set an additional http-header? I documented what I got when I tried what you're requesting here:

https://github.com/nelmio/NelmioApiDocBundle/pull/283/files#diff-d54056867729d3fa16536eed2bdd1715R196

Don't you get that error without having set the http-header?

@tonivdv
Copy link
Contributor Author

tonivdv commented Mar 11, 2014

I don't have any additional header, but I don't get an error, I just get the "Please reload the scheme" which is showed while browsing through https because my method is not explicitly marked as 'https'.

Hope you see what I mean.

@SimonSimCity
Copy link
Contributor

Tried it now. There's no downside for me - just that http-only requests do not work. But as @stof wrote earlier, it's quite unusual to set a request to be http-only ... So, I think this works out.

@tonivdv
Copy link
Contributor Author

tonivdv commented Mar 12, 2014

Thank you @SimonSimCity for checking.

@willdurand Can you merge and perform a bug fix release?

willdurand added a commit that referenced this pull request Mar 12, 2014
Sandbox should not be disabled in https whenever a method is not marked as https explictly
@willdurand willdurand merged commit eddba56 into nelmio:master Mar 12, 2014
@willdurand
Copy link
Collaborator

thx

@willdurand
Copy link
Collaborator

Done. 2.5.1

@tonivdv
Copy link
Contributor Author

tonivdv commented Mar 12, 2014

thx to you too ;)

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

Successfully merging this pull request may close these issues.

3 participants