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

fix: [CURLRequest] Multiple HTTP 100 return by API. #8466

Merged
merged 4 commits into from
Feb 17, 2024

Conversation

ping-yee
Copy link
Contributor

@ping-yee ping-yee commented Jan 27, 2024

Description
Closes #8347

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Jan 27, 2024
@michalsn
Copy link
Member

Since you were able to reproduce the issue, can you also provide a test case for this scenario? Thanks!

@michalsn michalsn added the tests needed Pull requests that need tests label Jan 27, 2024
@ping-yee
Copy link
Contributor Author

ping-yee commented Jan 28, 2024

@michalsn I'm afraid I can't do that, as this seems like a specific error that occurs when using a Python framework.

To write a test case for this scenario, we would need to set up a Flask API service in advance, which I don't think is a good direction.

However, I can provide the code I used when reproducing the issue.

Reproduce

In CI4

<?php

namespace App\Controllers;

use CodeIgniter\API\ResponseTrait;

class Home extends BaseController
{
    use ResponseTrait;

    public function index()
    {
        $client = \Config\Services::curlrequest();

        try {
            $options = [
                'headers' => [
                    'Expect' => '100-continue'
                ],
                'debug'   => true,
            ];

            $response = $client->request('GET', 'http://localhost:5000', $options);
            // We'll get 100 until we fix it.
            // After modification we will get 200.
            dd($response->getStatusCode());
        } catch (\Exception $e) {
            echo "Request failed: " . $e->getMessage();
        }
    }
}

In flask

from flask import Flask, jsonify

app = Flask(__name__)

@app.route('/')
def hello_world():
    return jsonify(message="Hello, World!")

if __name__ == '__main__':
    app.run(debug=True, host='0.0.0.0', port=5000)

Also, you can get the repeat response by using curl command.

curl localhost:5000 -v -H "Expect: 100-continue"
user@Py:/mnt/d/CodeIgniter4$ curl localhost:5000 -v -H "Expect: 100-continue" 
*   Trying 127.0.0.1:5000...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 5000 (#0)
> GET / HTTP/1.1
> Host: localhost:5000
> User-Agent: curl/7.68.0
> Accept: */*
> Expect: 100-continue
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 100 Continue
* Mark bundle as not supporting multiuse
< HTTP/1.1 100 Continue
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Server: Werkzeug/2.2.2 Python/3.7.17
< Date: Sun, 28 Jan 2024 06:05:36 GMT
< Content-Type: application/json
< Content-Length: 33
< Connection: close
<
{
  "message": "Hello, World!"
}
* Closing connection 0

@michalsn
Copy link
Member

We don't need Flask. We tested output before, like here: https://github.com/codeigniter4/CodeIgniter4/blob/develop/tests/system/HTTP/CURLRequestTest.php#L385

@kenjis
Copy link
Member

kenjis commented Jan 28, 2024

We use MockCURLRequest for testing:

return new MockCURLRequest(($app), $uri, new Response($app), $options);

@ping-yee
Copy link
Contributor Author

@michalsn @kenjis thank you for your explanation!
I have added the test case, review please.

@kenjis kenjis removed the tests needed Pull requests that need tests label Feb 13, 2024
@kenjis kenjis merged commit bc0517f into codeigniter4:develop Feb 17, 2024
62 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: [CURLRequest] Multiple HTTP 100 return by API
3 participants