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

PHPBrowser module: Wrong url sent and request does not match codeception debug output #50

Open
dpsarrou opened this issue Aug 9, 2018 · 7 comments

Comments

@dpsarrou
Copy link

dpsarrou commented Aug 9, 2018

What are you trying to achieve?

Assert that a file can be retrieved from a webserver using the REST module with PHPBrowser, by checking if the status code is equal to 200. The file exists on the server and can be retrieved using CURL with the same url as shown in the debug output of codeception.

What do you get instead?

Failed asserting that 404 matches expected 200 and wrong debug output of codeception.

Root cause

I've managed to successfully reproduce it and I think I've tracked down the issue:
My setup consists of docker images (nginx image proxying to php-fpm image) defined in a docker-compose.yml file as service web and app respectively. Codeception is run in separate docker image attached to the same network.

The problem seems to be the PHPBrowser url parameter as defined in the suite config file shown below. When there is no port defined and the url is http://web (the name of the nginx host since it listens to port 80) it sends a completely wrong url to the webserver. Instead of sending web/storage/avatars/... as described in the debug output of codeception the actual url received by nginx is me/web/storage/avatars/... You can see nginx logs below for more details. Now where did that me came from..??

It turns out that if you define a port in the PHPBrowser url variable, such as http://web:80 everything works as expected and the test passes. If you don't, like described above and for the specific test case I have posted below the url gets messed up and looks to be resolved from History urls.
Looking at the codeception src code the offending method seems to be in vendors symfony/browser-kit: https://github.com/symfony/browser-kit/blob/c55fe9257003b2d95c0211b3f6941e8dfd26dffd/Client.php#L322.

Looking at that method, specifically here: https://github.com/symfony/browser-kit/blob/c55fe9257003b2d95c0211b3f6941e8dfd26dffd/Client.php#L593 it seems that it tries to resolve the url based on History urls and since for my specific testcase I already have a previous http request sent in the same test, the url gets wrongly constructed.

Workaround

The workaround is, as shown above, to always define the port even if it is 80 or 443. I don't understand why the url needs to be constructed using previous history and I cannot say I agree with that approach but I also cannot suggest a fix at the moment.

Details

Provide console output if related. Use -vvv mode for more details.

Codeception output:

  [Page] /me/avatar
  [Response] 200
  [Request Cookies] []
  [Response Headers] {"Server":["nginx/1.15.0"],"Content-Type":["application/json"],"Transfer-Encoding":["chunked"],"Connection":["keep-alive"],"X-Powered-By":["PHP/7.2.8"],"Cache-Control":["no-cache, private"],"Date":["Thu, 09 Aug 2018 08:00:18 GMT"],"X-RateLimit-Limit":["200"],"X-RateLimit-Remaining":["197"],"X-Frame-Options":["SAMEORIGIN"],"X-XSS-Protection":["1; mode=block"],"X-Content-Type-Options":["nosniff"]}
  [Response] {"status":200,"payload":{"url":"web/storage/avatars/0f74205c-75f5-42f5-852e-48b32fb87820_gr95QH7P5RMuyzzEWysQngzb54Pj5CHyma3K1Ffk.jpeg","created_at":{"date":"2018-08-09 08:00:18.000000","timezone_type":3,"timezone":"UTC"},"updated_at":{"date":"2018-08-09 08:00:18.000000","timezone_type":3,"timezone":"UTC"}}}
 I see response code is 200
 I see response is json 
 I see response json matches json path "$.payload.url"
 I grab data from response by json path "$.payload.url"
 I assert not empty "web/storage/avatars/0f74205c-75f5-42f5-852e-48b32fb87820_gr95QH7P5RMuyzzEWysQngzb54Pj5CHyma3K1Ffk.jpeg"
 I send head "web/storage/avatars/0f74205c-75f5-42f5-852e-48b32fb87820_gr95QH7P5RMuyzzEWysQngzb54Pj5CHyma3K1Ffk.jpeg"
  [Request] HEAD web/storage/avatars/0f74205c-75f5-42f5-852e-48b32fb87820_gr95QH7P5RMuyzzEWysQngzb54Pj5CHyma3K1Ffk.jpeg []
  [Request Headers] {"Authorization":"Bearer token_reducted"}
  [Page] web/storage/avatars/0f74205c-75f5-42f5-852e-48b32fb87820_gr95QH7P5RMuyzzEWysQngzb54Pj5CHyma3K1Ffk.jpeg
  [Response] 404
  [Request Cookies] []
  [Response Headers] {"Server":["nginx/1.15.0"],"Content-Type":["application/json"],"Connection":["keep-alive"],"X-Powered-By":["PHP/7.2.8"],"Cache-Control":["no-cache, private"],"Date":["Thu, 09 Aug 2018 08:00:18 GMT"]}
  [Response] 
 I see response code is 200
 FAIL 

Nginx access log:

web_1     | 172.27.0.4 - - [09/Aug/2018:08:15:40 +0000] "GET /me/avatar HTTP/1.1" 200 324 "-" "Symfony BrowserKit"
web_1     | 172.27.0.4 - - [09/Aug/2018:08:15:40 +0000] "HEAD /me/web/storage/avatars/ec20bfa9-c252-43bb-b990-f97362445ec2_jBqWIZuRm62qJsXz1sAwfrrMvCzeps8ZXsrKxWZp.jpeg HTTP/1.1" 404 0 "http://web/me/avatar" "Symfony BrowserKit"

Provide test source code if related

    public function seeAvatarInfo(\AcceptanceTester $I)
    {
        $I->haveHttpHeader('Authorization', 'Bearer ' . $this->token);

        $I->sendGET('/me/avatar');
        $I->seeResponseCodeIs(200);
        $I->seeResponseIsJson();
        $I->seeResponseJsonMatchesJsonPath('$.payload.url');
        $url = $I->grabDataFromResponseByJsonPath('$.payload.url')[0];
        $I->assertNotEmpty($url);

        $I->sendHEAD($url);
        $I->seeResponseCodeIs(200);
    }

Details

  • Codeception version: 2.4.5
  • PHP Version: 7.2.8
  • Operating System: Docker (official php-fpm images) in OSX
  • Installation type: Docker (codeception image)
  • Suite configuration:
class_name: AcceptanceTester
modules:
    enabled:
        - Asserts
        - PhpBrowser:
            url: 'http://%APP_URL%'
        - REST:
            depends: PhpBrowser
@Naktibalda
Copy link
Member

How about making sure that your URL starts with '/' if you don't want it appended to previous URL?

@dpsarrou
Copy link
Author

dpsarrou commented Aug 9, 2018

Sure, but why even have a previous url appended anyway..? Seems counter-intuitive.

@Naktibalda
Copy link
Member

Because that is how relative URLs work.

@dpsarrou
Copy link
Author

dpsarrou commented Aug 9, 2018

No, relative URLs are relative to a specific base path, not random based on your previous history. In the output I've posted above it shows how random the creation is. My previous request was "/me/avatar" and the next one was "me/web/storage..". So it decided to only keep the "/me" part while according to your argument it should have been "/me/avatar/web/storage...". It just doesnt make sense.

@Naktibalda
Copy link
Member

It is absolutelly correct, you just don't get it.
Relative URLs behave the same as relative paths in filesystem.

Since there is no / at the end of /me/avatar , avatar is a filename and /me/ is a directory,
if you click a link to storage, browser assumes that it is another file in the same directory and makes request to /me/storage.

https://www.w3.org/TR/WD-html40-970917/htmlweb.html#h-5.1.2

@dpsarrou
Copy link
Author

dpsarrou commented Aug 9, 2018

I'm sorry, is this issue about the definition of relative/absolute urls or an unexpected behavior of codeception that is not documented anywhere..? Are you going to focus on that or not..? In the documentation https://codeception.com/docs/modules/REST under sendGET/HEAD sections it clearly says:

Sends a GET request to given uri.

It does not talk about any relative uri, and I even would accept it if it was relative to the base url you provide in the configuration of the REST module, which in my case is http://web. What does my previous request history have to do with this..? Why did it even decide to make a request relative to whatever path I had requested before..? How is this expected and even logical..? And why does codeception mention in the debug output a URI that was NOT the one that was sent to the server..?

These are the issues, not the definitions.

@eXorus eXorus transferred this issue from Codeception/Codeception Jan 18, 2021
@ItsReddi
Copy link

Discovered this feature also, while sending multiple delete requests, one after another.

        //delete as reader - no access to delete
        $I->amBearerAuthenticated(self::VALID_BEARER_READER);
        $I->sendDelete('tasks/86');
        $I->canSeeResponseCodeIs(HttpCode::NOT_FOUND);

        //delete as assigner - no access to delete
        $I->amBearerAuthenticated(self::VALID_BEARER_ASSIGNER);
        $I->sendDelete('tasks/86');
        $I->canSeeResponseCodeIs(HttpCode::NOT_FOUND);

Second delete would be a DELETE on:
http://localhost/tasks/tasks/86
That was more than unexpected.

And the test is false positive.

While i understand the relative/absolute url argumentation, it should be at least documented for security.

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

No branches or pull requests

3 participants