NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[Profiler] Fix profiler nullable string type#36162

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

fabpot merged 1 commit intosymfony:5.0frommRoca:fix_profile_set_ip_type

Apr 21, 2020

Merged

[Profiler] Fix profiler nullable string type#36162

fabpot merged 1 commit intosymfony:5.0frommRoca:fix_profile_set_ip_type

Apr 21, 2020

Conversation

Copy link

Contributor

mRoca commentedMar 21, 2020•
edited
Loading

Q	A
Branch?	5.0
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets	n/a
License	MIT
Doc PR	-

This PR fixes nullable string types in setter for the Profile class.

The detected issue comes fromthe Profiler class :

$profile->setIp($request->getClientIp());// string or null

The corresponding return types for the Profile getters are allready good:

/**     * Returns the IP.     *     * @return string|null The IP     */publicfunctiongetIp()    {return$this->ip;    }

Fix profiler nullable string type

b5d4061

carsonbot added Status: Needs Review WebProfilerBundle Bug labels

Mar 21, 2020

Copy link

Member

Kocal commentedMar 21, 2020

Or maybe we can call->setIp() only when$request->getClientIp() is notnull?

nicolas-grekas added this to the5.0 milestone

Mar 23, 2020

Copy link

Member

nicolas-grekas commentedMar 23, 2020

Setting the IP or URL to null doesn't look useful from the pov of the API, does it? How did you spot this?

Copy link

ContributorAuthor

mRoca commentedMar 23, 2020

I've spotted it by calling the fpm agent withcgi-fcgi (for container cache warmup & docker healthcheck):

REDIRECT_STATUS=true SCRIPT_FILENAME=/srv/public/index.php HTTP_HOST=127.0.0.1 REQUEST_URI=/ REQUEST_METHOD=GET HTTP_USER_AGENT=probe cgi-fcgi -bind -connect 127.0.0.1:9001

This line command won't work because of this type bug, as$request->getClientIp() parses the$_SERVER['REMOTE_ADDR'] value. But if I add the corresponding headerREMOTE_ADDR=127.0.0.1 to the command, everything is fine.