Skip to content

Set curl option follow location to true #31

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

Closed
wants to merge 1 commit into from
Closed

Set curl option follow location to true #31

wants to merge 1 commit into from

Conversation

olleharstedt
Copy link
Contributor

This is to allow redirect from http to https.

This is to allow redirect from http to https.
@olleharstedt
Copy link
Contributor Author

olleharstedt commented Feb 21, 2018

Notice that the returned string is slightly different when redirect from HTTP to HTTPS is used. Instead of just a GIF, I got

HTTP/1.1 200 OK
Server: nginx/1.10.3 (Ubuntu)
Date: Wed, 21 Feb 2018 09:31:38 GMT
Content-Type: image/gif
Transfer-Encoding: chunked
Connection: keep-alive

GIF89a\000\000\000\000\000\000\000\000\000\000!�\000\000\000\000,\000\000\000\000\000\000\000D\000;

Without redirect (using https in the URL) I got:

GIF89a\000\000\000\000\000\000\000\000\000\000!�\000\000\000\000,\000\000\000\000\000\000\000D\000;

@tsteur
Copy link
Member

tsteur commented Feb 21, 2018

Awesome thank you 👍 I checked it should work for all PHP versions.

One possible thing to think about: What do we do when there is a location redirect and it is a POST request? The POST request would not re-submit the data AFAIK and the tracking request would not work. Maybe for now we enable it only when $method === 'GET'?

This is the case in all requests but bulk tracking. For POST requests I wonder if we should later rather implement to check if there is a location header in the response, and if so POST the request to that URL? (can be done in a separate PR/Issue)

@tsteur
Copy link
Member

tsteur commented Jun 19, 2019

@olleharstedt are you still working on this PR? Could we add the follow option maybe only for GET requests?

@olleharstedt
Copy link
Contributor Author

Hi! Was on vacation. Yes, I can modify the PR to include the option only for GET requests.

@olleharstedt
Copy link
Contributor Author

New PR here: #46

@innocraft-automation innocraft-automation removed this from the Current sprint milestone Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants