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

Bug: prototype of setBody($data); method in CodeIgniter\HTTP\Message should be changed to setBody(string $data); #2466

Closed
SunilEver opened this issue Dec 28, 2019 · 5 comments
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@SunilEver
Copy link

Describe the bug
prototype of setBody($data); method in CodeIgniter\HTTP\Message should be changed to setBody(string $data); as when calling IncomingRequest::getJson() the method finds something other string and tries to json decode it.

CodeIgniter 4 version
v4.0.0-rc.3

Affected module(s)
All request modules.

Expected behavior, and steps to reproduce if appropriate
CodeIgniter\HTTP\Message::setBody should throw an error if anything other than string is passed.

Context

  • OS: Ubuntu 16
  • Web server php native server
  • PHP version 7.3.13
@SunilEver SunilEver added the bug Verified issues on the current code behavior or pull requests that will fix them label Dec 28, 2019
@SunilEver SunilEver changed the title Bug: Bug: prototype of setBody($data); method in CodeIgniter\HTTP\Message should be changed to setBody(string $data); Dec 28, 2019
@lonnieezell
Copy link
Member

I believe it's perfectly valid to set an array or object as the body of a Response (which extends Message) so the suggested change would cause issues elsewhere.

Please provide enough details to recreate the issue that you're having.

@SunilEver
Copy link
Author

SunilEver commented Jan 2, 2020 via email

@lonnieezell
Copy link
Member

I'm a bit confused here. The IncomingRequest's getJSON is intended to retrieve the HTTP request's body and parse it as JSON. That will always be a string after going through a web server, I believe.

You said you have problems with setting a value in the Message body. How does this have anything to do with IncomginRequest? It sounds like you might be using it a way it's not intended for.

@SunilEver
Copy link
Author

SunilEver commented Jan 13, 2020 via email

@MGatner
Copy link
Member

MGatner commented Jan 30, 2020

We have at least one test that explicitly uses setBody() with an array:

	public function testJSONGetFromNormalBody()
	{
...
		$body     = [
			'foo' => 'bar',
...
		];
...
		$response->setBody($body);

		$this->assertEquals($expected, $response->getJSON());
	}

I think the type needs to remain undefined, but I will update the header variable type so it doesn't specify string.

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

No branches or pull requests

3 participants