Drupal can be "consumed" from other context than classic browsers. This can be drush request or cURL request for which there will likely be no "Referer" header in the HTTP request, but it can also happen to be from applications which set non standard URI in the Referer field.

For example, Adobe AIR applications sets a Referer header with the following pattern : air:/[filename].swf and Drupal duly store this URI in the "referer" field of dblog entries.

The issue is that when trying to render such dblog entries (at admin/reports/dblog/event/[id]), it tries to treat theses URI as routable URL and render theses as links, which fails :

InvalidArgumentException: The URI 'app:/[filename].swf' is invalid. You must use a valid URI scheme. Use base: for a path, e.g., to a Drupal file that needs the base path. Do not use this for internal paths controlled by Drupal. in Drupal\Core\Utility\UnroutedUrlAssembler->assemble() (line 64 of /project/www/core/lib/Drupal/Core/Utility/UnroutedUrlAssembler.php).

Whether it's good or bad to but put such bizarre URI in the Referer header of such request is out of scope for this issue as Drupal should always be able to render any of its dblog entries. Moreover, having this piece of information in dblog entries is very valuable to understand that the request comes from such an application.

I think that Drupal should keep on storing this kind of non-standard URI in dblog entries and output them as plain text and not as a link.

This issue is very close to #2755497: InvalidArgumentException in dblog entries due to invalid location which try to solve a similar issue for the "Location" field.

CommentFileSizeAuthor
#15 exception_thrown_when-2858182-14.patch1.31 KBcilefen
#9 exception_thrown_when-2858182-9.patch706 bytesgarphy

Comments

garphy created an issue. See original summary.

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented

I think that Drupal should keep on storing this kind of non-standard URI in dblog entries and output them as plain text and not as a link.

Well if the text is a valid URL we can display it as a link. UrlHelper::isValid can help with that.

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented
Issue tags: +Novice

Tentativelly marking this as Novice.

Plan:

  • In DbLogController::eventDetails() Check if the url is valid before render it as a link.
  • Create a new method test in the DbLogTest to create a log entry with a referer like mentioned in the issue summary. You can use DbLogTest::testLogEventPage() as base.
  • Assert in that method that the text is displayed as a text link.
cilefen’s picture

cilefen commented
Title: Issue with dblog entries created from request containing non-routable URI in referer header » Exception thrown when viewing dblog entries created from requests containing non-routable URIs in referer header
Priority: Normal » Major
Related issues: +#2855791: Bad link in aggregator crashes production website

This is major priority according to the guidelines. I related an issue where a "bad" URI can appear from an external source.

garphy’s picture

garphy
Primary language French
Location Montreal, Canada
commented
Assigned: Unassigned » garphy
garphy’s picture

garphy
Primary language French
Location Montreal, Canada
commented

I think that the issue root is actually from an incorrect behavior of UrlHelper :
* UrlHelper::isValid('air:/test.swf') === TRUE
* UrlHelper::isExternal('air:/test.swf') === FALSE

So we can't really rely on UrlHelper::isValid()here because UrlHelper::isExternal() result will lead UnroutedUrlAssember::assemble() to fail.

Either air:/test.swf is an invalid URI, or it has to be considered as an external URL.

IMHO, UrlHelper::isExternal('air:/test.swf') should be TRUE. Thoughts ?

cilefen’s picture

cilefen commented
Issue tags: -Novice

If there is ambiguity, I remove the Novice tag until there is concensus. A test proving the issue would be a good start.

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented

Thanks @garphy. UrlHelper::isExternal seems appropriated.

garphy’s picture

garphy
Primary language French
Location Montreal, Canada
commented
Status: Active » Needs work
StatusFileSize
new exception_thrown_when-2858182-9.patch706 bytes

Here's an update of UrlHelperTest which exhibits that air:/filename.swf is not considered an external URL.
Testbot should fail.

garphy’s picture

garphy
Primary language French
Location Montreal, Canada
commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 9: exception_thrown_when-2858182-9.patch, failed testing.

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented
Assigned: garphy » Unassigned

Maybe someone else can help with this issue. Unassigned for now.

cilefen’s picture

cilefen commented

This can be fixed by copying default.services.yml to services.yml and adding 'air' to parameters.filter_protocols. That is not a solution to invalid referrer lines throwing an exception, of course.

garphy’s picture

garphy
Primary language French
Location Montreal, Canada
commented

This can be fixed by copying default.services.yml to services.yml and adding 'air' to parameters.filter_protocols. That is not a solution to invalid referrer lines throwing an exception, of course.

Maybe we can "detect" that the provided referer don't match any of the protocols listed in services.yml and render it as plain text instead of a link ?

cilefen’s picture

cilefen commented
Status: Needs work » Needs review
StatusFileSize
new exception_thrown_when-2858182-14.patch1.31 KB

This demonstrates that in Drupal 8, you can create a Url that cannot be generated.

cilefen’s picture

cilefen commented

The test in #9 illustrates the issue to a degree, but because of #13, it is not a representative test. I suggest this improvement: causing or creating a dblog event with a "bad" referer then trying a page load.

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented

I just created this: #2868725: Refactor how dblog module is rendering links in event details

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented
Status: Needs review » Closed (duplicate)

I'm marking this as a duplicate of #2868725: Refactor how dblog module is rendering links in event details. We should work in a generic solution for several issues we have related the way dblog render links.