Fixed: File::uri() is not consistent with other Entity types.

[mikemccaffrey]

As part of building the Link formatter for the Reference module, we encountered a problem where the $entity->uri() function for file entities includes a stream wrapper identifier in the beginning like "public://field/image/imagefield_1g3q2W.png".

Right now, it tries encoding the "public://" into the url itself, creating a bad link.

Option 1: We should add a check in the url() function to check if a path starts with stream wrapper identifier, and format the resulting url using the stream wrapper.
Option 2: We should change File::uri() to be consistent with other entity types, and link to file/[fid] page instead of a direct path to the file itself.

At the time this issue was originally created, Files did not yet have pages. Now that they do, we have two options to solve this problem.

[quicksketch]

I took a look at this problem and unfortunately it gets messy. While I think it makes sense to make url() support stream wrappers, we have some problems making this possible.

I tried just putting file_create_url() in url() like this:

  // Convert file URIs into URLs. This always results in an external URL. It may
  // be shorted back into a relative one below.
  $scheme = file_uri_scheme($path);
  if ($scheme && file_stream_wrapper_valid_scheme($scheme)) {
    $path = file_create_url($path);
  }

But we have methods like BackdropPrivateStreamWrapper::getExternalUrl() that call url() inside of them. This causes either double-encoding and/or double unclean URL handling. So instead of getting a path like ?q=system/files/example.png you get /?q=%3Fq%3Dsystem/files/example.png.

The best idea I can come up with is introducing a new method on stream wrapper classes that returns the Backdrop internal path if possible. e.g. "files/example.png" or "system/files/example.png", that could then be passed into url() safely. If this method returned FALSE, then the absolute URL returned by getExternalUrl() would be used instead.

[quicksketch]

Making a new method like getBackdropPath() also has new problems. For a public:// file path, if we returned "files/example.png" and clean URLs were turned off, running through url() would result in /?q=files/example.png, which we also don't want.

[jenlampton]

Is this a blocker for reference module? Or just something that's related?

[jenlampton]

No PR here yet, pushing to 1.7

[jenlampton]

As this issue has already missed a core release and still doesn't have a PR, I'm going to remove the milestone. If we determine that this is a priority (a blocker for reference module?) We can reschedule it as soon as it has a PR.

[quicksketch]

I updated the issue summary here to better reflect the options we have. Now that files have menu paths (i.e. file/[fid]) we also have the option of making File::uri() consistent with other entity types, rather than making the wider-impacting change of modifying url() and l() to support file paths.