WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#17682 closed defect (bug) (fixed)

Browser nag PHP Notice if no user agent is set

Reported by: duck_ Owned by: ryan
Milestone: 3.2 Priority: normal
Severity: normal Version: 3.2
Component: Warnings/Notices Keywords: has-patch
Focuses: Cc:

Description

Not too likely to happen in practice, but it's possible and I ran across it today.

Notice: Undefined index: HTTP_USER_AGENT in /srv/www/wp.dev/public/wp-admin/includes/dashboard.php on line 1201
Notice: Undefined index: HTTP_USER_AGENT in /srv/www/wp.dev/public/wp-admin/includes/dashboard.php on line 1207 

Patch has added benefit of not hitting the .org API if there is no user agent to send.

Attachments (3)

isset-http_user_agent.17682.diff (474 bytes) - added by duck_ 3 years ago.
17682.2.diff (2.6 KB) - added by duck_ 3 years ago.
17682.diff (5.0 KB) - added by aaroncampbell 3 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 follow-up: ryan3 years ago

wp_check_browser_version() returns false in one spot and now void in two spots. Should those all be returning false? The three calls core makes to wp_check_browser_version() could stand to check for false.

duck_3 years ago

comment:2 in reply to: ↑ 1 duck_3 years ago

Replying to ryan:

wp_check_browser_version() returns false in one spot and now void in two spots. Should those all be returning false? The three calls core makes to wp_check_browser_version() could stand to check for false.

Agreed. Standardised on returning false. Added in checks for truthy-ness of the returned value too even though it's not entirely necessary.

Also added @return docs, added spacing in inline response documentation and added standard .0 to 3.2.

aaroncampbell3 years ago

comment:3 aaroncampbell3 years ago

I had worked up a patch too. Attaching just in case. It looks like the main difference is that in wp_dashboard_browser_nag I wrapped all of the content creating in an if( $response ) since it uses $response a couple times in content creation. If we go witht eh .2 patch from duck we need to check the variables being passed into the sprintf as well. My patch simply passes an empty string to the browse-happy-notice filter if wp_check_browser_version() returns false.

I also added @return docs, but I didn't catch the 3.2.0 vs 3.2 thing, so if we go with my patch we should probably add that.

comment:4 ryan3 years ago

  • Owner set to ryan
  • Resolution set to fixed
  • Status changed from new to closed

In [18150]:

Check return value of wp_check_browser_version(). Make return value consistent. Props duck_, aaroncampbell. fixes #17682

Note: See TracTickets for help on using tickets.