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

Don't set empty body on external curl requests [Fixes R4791] #554

Closed

Conversation

enov
Copy link
Contributor

@enov enov commented Sep 19, 2014

Originally opened by @acoulton in R4791 and based on #403

When the CURLOPT_POSTFIELDS option is present, curl adds a
default Content-Type header which can be changed but not
removed, causing authentication problems with signed requests.
The option should only be set if a request body is being sent.

Ref #547. Please review and merge :)

Thanks!

When the CURLOPT_POSTFIELDS option is present, curl adds a
default Content-Type header which can be changed but not
removed, causing authentication problems with signed requests.

The option should only be set if a request body is being sent.
@acoulton
Copy link
Member

@enov I was confused by this, and have only just realised that 3.2/master hasn't been merged up into 3.3/develop for a while (3.3/develop...3.2/master).

Shouldn't we just be doing that - since the branching policy is that bugfixes etc get applied to the earliest supported version and then merged up?

Otherwise we're going to need to go through every commit in that diff and check whether or not it needs to be cherry-picked into the 3.3 series....

@enov
Copy link
Contributor Author

enov commented Sep 19, 2014

Well, since the files were renamed for PSR-0 in 3.3, I thought git would not handle that? If there is a way, why not?

Also, @shadowhand stated here that this should be done manually.
http://discourse.kohanaframework.org/t/search-for-collaborators/56/37

There is no automatic process to merge things from 3.2 into 3.3, it has to be done manually

@acoulton
Copy link
Member

@enov git's smart enough to cope with most of the renames (almost all of them are committed as renames rather than delete/add).

There are obviously a bunch of merge conflicts because the two branches have diverged so much.

I believe @shadowhand meant that there's no CI or person dedicated to merging up the branches, not that the fixes had to be cherry picked up.

If you browse back in the 3.3/develop history there are quite a few merge commits from 3.2 to 3.3 eg 1f05385, 5c4d74a - ISTR (and from the history it looks like) the maintainer workflow used to be:

  • merge PR / commit fix to 3.2/develop
  • checkout 3.3/develop
  • merge 3.2/develop to 3.3/develop
  • deal with any merge conflicts
  • if the 3.2 fix was not valid for 3.3 (eg because it's a bug that doesn't exist any more because of refactoring, or because it needed to be solved differently) then make those changes in 3.3/develop by reverting or modifying the older solution. Sometimes this was squashed in as part of the merge commit eg by removing lines/files from the patch.

That way the 3.3/develop branch always stayed ahead of the 3.2/master branch rather than being divergent as it is now.

It probably makes sense to work gradually through the branch difference and try to merge up in smaller groups as they would have been in the past, rather than having one monster merge commit with all the conflicts bunched in together.

I still think that's going to be better than cherry-picking one by one and will make sure we don't miss anything unless we explicitly don't want it. We should also be making sure that we're merging 3.3 up to 3.4 on an ongoing basis to avoid them drifting apart.

acoulton added a commit that referenced this pull request Sep 26, 2014
…titute_character

Merge bugfixes from 3.2/master up to 3.3/develop.

Also closes #557 (an earlier version of this pull) and 
closes #554 (a bugfix that was already applied in 3.2 
and is merged up with this pull request.
@acoulton
Copy link
Member

Closed by #558 - for some reason github didn't detect my merge commit commit.

@acoulton acoulton closed this Sep 26, 2014
@enov enov deleted the 3.3/bugfix/R4791/curl-sets-content-type-on-request branch September 26, 2014 08:10
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.

2 participants