Problem/Motivation

In the pathologic.module there is a fromUri.
fromUri() can throw an exception, we shouldn't die on that.

Proposed resolution

Add a try-catch bloc there.

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

edurenye created an issue. See original summary.

edurenye’s picture

Status: Active » Needs review
FileSize
734 bytes

Added a try-catch bloc.

Garrett Albright’s picture

It looks like the exception is only thrown if parse_url() fails to find a scheme in the URL, but since we're prefixing what we pass in there with "base://", I don't see how that can ever happen. Am I missing something?

Berdir’s picture

I don't remember what the exact URL was that caused it but I did get an exception on a production site with a lot of weird migrated content so I added this as a way out, seemed better than an aborted page request :)

One way to break it is "base:2015/10/10" because parse_url() parses that incorrectly, that's another problem I had but not with pathologic. But this module calls it with base:// (note that the first is apparently actually the correct way to call it, according to core's usage of base:), then it doesn't break.

Found one way that breaks it: \Drupal\Core\Url::fromUri('base:///') but I guess it's possible that all that url validation and parsing above removes leading /, don't remember exactly.

I'll try if I can find what pattern broke it exactly.

hey_germano’s picture

I ran into this one last night. One of our clients had put some incorrect HTML in a block on their homepage, like this:

<a href="From left to right: Monique Coombs - Maine Coast Fishermen’s Association, Taylor Witkin - Eating with the Ecosystem, and Alan Lovewell - Real Good Fish talk about values">blog post</a>

When I set this up locally to investigate and turned on error reporting, I was able to see the full trace - https://gist.github.com/sarahg/6d53977c446af431777c

The patch in #2 keeps the page from crashing. Thanks!

Berdir’s picture

Status: Needs review » Reviewed & tested by the community

Yes, it is weird URL's like that.

Garrett Albright’s picture

Status: Reviewed & tested by the community » Fixed
Garrett Albright’s picture

Status: Fixed » Reviewed & tested by the community

Committed, but not pushed yet (I need to link my local repo from the one on D.o again and I don't have time to do that now), so I'll set back to RTBC.

  • edurenye authored 4c3a9b5 on 8.x-1.x
    Issue #2602312 by edurenye, Berdir: Catch exception in fromURI
    
  • a322e65 committed on 8.x-1.x
    Add test to recognize failure shown in Issue #2602312 (not catching...
Garrett Albright’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.