get_page_link() doesn't check if a valid post object is returned before trying to access its properties

[rodrigosprimo]

If get_page_link() receives an invalid post ID as the first parameter it will generate a PHP notice:

$ wp shell
>>> get_page_link( -1 )
PHP Notice:  Trying to get property of non-object in /vagrant/www/wccore/htdocs/wp-includes/link-template.php on line 317

Should a check be added to prevent this notice from happening? I'm happy to submit a patch. I'm just not sure what is the best way to handle this. Should the function return null if get_post() can't find a valid post? Should it call _doing_it_wrong()? Something else?

…properties

Modified get_page_link() function to check if post object is found before proceeding to access its properties.

Trac ticket: 44497

[westonruter]

I think we can simply update get_page_link() to return an empty string if get_post() fails to return a WP_Post. We can skip the _doing_it_wrong(), I think.

Also, the same should be done for _get_page_link().

[rodrigosprimo]

Hi @westonruter, thanks for your input on this ticket!

Is there a particular reason you suggested returning an empty string? I'm asking because I'm wondering if returning false would be better. It is a clearer signal to callers that something went wrong, and it would be consistent with other functions in the same file, like get_permalink(), get_post_permalink(), and wp_get_canonical_url(), which all return false when the post is invalid.

That said, returning false would technically change the current behavior. Right now, the function returns a truthy string ("?page_id=" appended to the home URL) accompanied by PHP warnings. However, since that string is a broken URL, any code that depends on its truthiness would already produce broken links, so I guess the practical impact is minimal. But still something to consider.

While reviewing link-template.php to work on this fix, I noticed that a few other functions in the same file have the same issue as get_page_link() and _get_page_link():

• permalink_anchor()
• get_attachment_link()
• the_shortlink()

Would you prefer that I address those in this same ticket, in a separate ticket, or are they not worth fixing?