f3-factory / fatfree-core

Fat-Free Framework core library
GNU General Public License v3.0
207 stars 88 forks source link

Fix HEADERS hive key not being populated when using \Base::mock() #261

Closed nanawel closed 2 years ago

nanawel commented 6 years ago

Although $f3->get('SERVER.HTTP_[...]') is able to return the correct result while mocking requests, it's not the case with $f3->get('HEADERS.[...]').

This PR just sets this key with the specified headers in the mock() method.

ikkez commented 6 years ago

yes possible, but IMO it should only set it when at least some headers were given (not null).

nanawel commented 6 years ago

You're perfectly right. I've adapted the code accordingly.

ikkez commented 6 years ago

No that's not what I mean. It still overrides the HEADERS, when no headers were given.

nanawel commented 6 years ago

Well, when a requests comes in, it should indeed override all the headers. At least that's the behavior that seems much closer to the real processing of a request to me.

But I agree this is not something you'll want the framework to force. Just like it does not override all the SERVER.HTTP_[...] keys when processing the specified $headers. I'll make the changes.

nanawel commented 6 years ago

Wait, I've come into an issue where I cannot clear correctly the HEADERS key because of

        $this->hive=['HEADERS'=>&$headers];

in \Base constructor.

It does not reset properly the key between tests because of the reference. I'm searching for a solution.

nanawel commented 6 years ago

I have a small test case that shows how HEADERS in request mocks can be considered still a bit buggy even with this PR:

*This works (using `SERVER.HTTP_` variables to access headers):**

$fw = \Base::instance();
$test = new \Test(\Test::FLAG_False);

class MyHandler {
    function foo(\Base $fw) {
        echo 'foo' . $fw->get('SERVER.HTTP_MYHEADER');
    }
}

$fw->route('GET foo', 'MyHandler->foo');

$fw->mock('GET foo'); // prints "foo"
$test->expect('foo' === $fw->get('RESPONSE'), '#1 (GET foo)');

$fw->mock('GET foo', [], ['MYHEADER' => 'BAZ']); // prints "fooBAZ"
$test->expect('fooBAZ' === $fw->get('RESPONSE'), '#2 (GET foo w/ header)');

$fw->clear('SERVER');

$fw->mock('GET foo'); // prints "foo" again
$test->expect('foo' === $fw->get('RESPONSE'), '#3 (GET foo)');

var_dump($test->results());
array(0) { // OK, no failed tests
}

While this doesn't (using HEADERS):

$fw = \Base::instance();
$test = new \Test(\Test::FLAG_False);

class MyHandler2 {
    function foo(\Base $fw) {
        echo 'foo' . $fw->get('HEADERS.MYHEADER');
    }
}

$fw->route('GET foo', 'MyHandler2->foo');

$fw->mock('GET foo'); // prints "foo"
$test->expect('foo' === $fw->get('RESPONSE'), '#1 (GET foo)');

$fw->mock('GET foo', [], ['MYHEADER' => 'BAZ']); // prints "fooBAZ"
$test->expect('fooBAZ' === $fw->get('RESPONSE'), '#2 (GET foo w/ header)');

$fw->clear('HEADERS');

$fw->mock('GET foo'); // prints "fooBAZ" => KO
$test->expect('foo' === $fw->get('RESPONSE'), '#3 (GET foo)');

var_dump($test->results());
array(1) {
  [0] =>
  array(3) {
    'status' =>
    bool(false)
    'text' =>
    string(12) "#3 (GET foo)"
    'source' =>
    string(61) ".../tests/test.ut.php:66"
  }
}

And this is because of the clear() method using \Base::init field to restore the original value, and the HEADERS subkeys being linked by reference in the __construct() (at least this is how I interpret it).

Maybe I should file a bug report for this. What do you think?

nanawel commented 6 years ago

Still not found a way to fix this bug here, but I've also added a new commit to fix another issue: when using mock(), the Content-Type and Content-Length headers are populated to the SERVER hive with the HTTP_ prefix, while they shouldn't. See how it's done in __construct() for example.

nanawel commented 2 years ago

Closing this PR.