Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explicit marker to rewrite URLs in assets #25

Closed
rgoldberg opened this issue Jul 16, 2015 · 7 comments
Closed

Explicit marker to rewrite URLs in assets #25

rgoldberg opened this issue Jul 16, 2015 · 7 comments

Comments

@rgoldberg
Copy link
Contributor

I'd like to add support for an explicit marker to rewrite URLs in assets.

I have some assets that will be easier to reference via absolute URLs, and AP currently only rewrites relative URLs; it doesn't rewrite absolute URLs.

I'm working on this in a branch in my AP fork.

I will also include some minor modifications that should improve the existing AP Processor code (factoring out duplicate code, simplifying & hopefully speeding up regex matching, properly handling URL etc.).

I'll create a pull request once the code is ready.

I propose to still rewrite relative URLs in both css & html files as before. I'll probably add URL rewriting capability for js files, too, if you don't mind.

The new code will also rewrite any URL that begins with "asset:", regardless if it's relative or absolute.

If the URL following "asset:" is:

  1. relative, URL rewriting will behave as for normal relative URLs
  2. absolute, sans host name, URL rewriting will still prepend the configured base URL, and will insert digest fingerprints like before. The path to the asset will be as specified in the absolute URL, and not be relative to the base file
  3. absolute, with host name, URL rewriting will NOT prepend the configured base URL, but will still insert digest fingerprints like before. The hostname & path to the asset will be as specified in the absolute URL, and not be relative to the base file

How should URL schemes for relative URLs be handled? (e.g., in an asset file located at http://host/path/asset.html, there is a relative URL like ftp:this/is/a/relative/path; I ignore cache digests for these examples)

If no base URL has been specified, then I imagine that we could use the hostname of the asset file, and make the path relative to the path of the referring file. We could use the scheme from the relative URL (in the case of the example, the rewritten URL will be ftp://host/path/this/is/a/relative/path).

The only problem is if a base URL has been specified and it has a scheme (e.g., http://cdn/base/path). Can we replace the scheme from the base URL with the scheme from the relative URL (so, for the example, we'd use ftp://cdn/base/path/this/is/a/relative/path)?

In the future, if you want to allow AP to be separately configured to enable / disable relative URL rewriting, and to enable / disable "asset:" scheme URL rewriting, I'll leave that config setup to you.

@davydotcom
Copy link
Contributor

That makes sense . In core we actually have a registered URL protocol for asset:

Sent from my iPhone

On Jul 16, 2015, at 7:44 PM, rgoldberg notifications@github.com wrote:

I'd like to add support for an explicit marker to rewrite URLs in assets.

I have some assets that will be easier to reference via absolute URLs, and AP currently only rewrites relative URLs; it doesn't rewrite absolute URLs.

I'm working on this in a branch in my AP fork.

I will also include some minor modifications that should improve the existing AP Processor code (factoring out duplicate code, simplifying & hopefully speeding up regex matching, properly handling URL etc.).

I'll create a pull request once the code is ready.

I propose to still rewrite relative URLs in both css & html files as before. I'll probably add URL rewriting capability for js files, too, if you don't mind.

The new code will also rewrite any URL that begins with "asset:", regardless if it's relative or absolute.

If the URL following "asset:" is:

relative, URL rewriting will behave as for normal relative URLs
absolute, sans host name, URL rewriting will still prepend the configured base URL, and will insert digest fingerprints like before. The path to the asset will be as specified in the absolute URL, and not be relative to the base file
absolute, with host name, URL rewriting will NOT prepend the configured base URL, but will still insert digest fingerprints like before. The hostname & path to the asset will be as specified in the absolute URL, and not be relative to the base file
How should URL schemes for relative URLs be handled? (e.g., in an asset file located at http://host/path/asset.html, there is a relative URL like ftp:this/is/a/relative/path; I ignore cache digests for these examples)

If no base URL has been specified, then I imagine that we could use the hostname of the asset file, and make the path relative to the path of the referring file. We could use the scheme from the relative URL (in the case of the example, the rewritten URL will be ftp://host/path/this/is/a/relative/path).

The only problem is if a base URL has been specified and it has a scheme (e.g., http://cdn/base/path). Can we replace the scheme from the base URL with the scheme from the relative URL (so, for the example, we'd use ftp://cdn/base/path/this/is/a/relative/path)?

In the future, if you want to allow AP to be separately configured to enable / disable relative URL rewriting, and to enable / disable "asset:" scheme URL rewriting, I'll leave that config setup to you.


Reply to this email directly or view it on GitHub.

@rgoldberg
Copy link
Contributor Author

Thanks.

I'm not exactly sure about what you mean by "In core we actually have a registered URL protocol for asset:"

Do you mean that java.net.URL will not consider asset an unknown protocol?

Or do URLs with an "asset" scheme (scheme == protocol) already get processed at all by AP?

Or both? Or something else?

I don't want to break things by processing them differently in my new code than in the existing code.

BTW, I don't have much experience with git, but it seems like something might be amiss with the rel-2.3.8 tag. The code in the rel-2.3.8 tag doesn't appear to be in the master branch (I'm used to mercurial, where a tag just points to changeset within a branch).

When I git checkout tags/rel-2.3.8, then run git status, I see HEAD detached at rel-2.3.8 in the status output. I don't know if this is normal.

My new code will be in a branch named ExplicitUrlRewriteMarker that is branched from the rel-2.3.8 tag. Please let me know if I should branch from elsewhere.

Thanks.

@rgoldberg
Copy link
Contributor Author

Is the base URL from the grails.assets.url Grails config property ever passed from asset-pipeline to asset-pipeline-core?

If so, is it available in the runtime of `Processor#process(String inputText, AssetFile assetFile)?

I haven't had the time to track it down.

The existing HtmlProcessor does not prepend the value of grails.assets.url in rewritten URLs.

Some of my static files in S3 are partials that are loaded by GSPs in my Grails server.

From what I know, URLs in the partials will be relative to the GSP's URL.

This URL in on my Grails server, not on S3, so that URLs rewritten in the partials will point to the Grails server, unless I can somehow prepend the value of grails.assets.url.

Thanks.

@davydotcom
Copy link
Contributor

AssetPipelineConfigHolder.config.url

Sent from my iPhone

On Jul 17, 2015, at 2:52 AM, rgoldberg notifications@github.com wrote:

Is the base URL from the grails.assets.url Grails config property ever passed from asset-pipeline to asset-pipeline-core?

If so, is it available in the runtime of `Processor#process(String inputText, AssetFile assetFile)?

I haven't had the time to track it down.

The existing HtmlProcessor does not prepend the value of grails.assets.url in rewritten URLs.

Some of my static files in S3 are partials that are loaded by GSPs in my Grails server.

From what I know, URLs in the partials will be relative to the GSP's URL.

This USL in on my Grails server, not on S3, so that URLs rewritten in the partials will point to the Grails server, unless I can somehow prepend the value of grails.assets.url.

Thanks.


Reply to this email directly or view it on GitHub.

@rgoldberg
Copy link
Contributor Author

Thanks for the info about AssetPipelineConfigHolder.config.url.

I'm almost done with my changes for this issue, so I'll have a pull request soon.

Does the following list accurately describe how grails.assets config properties affect how asset:* tags in GSPs (e.g., asset:image, asset:javascript, asset:stylesheet, etc.) generate asset URLs?

  1. url specifies the base URL for assets; if both url & mapping are supplied, url overrides mapping, and mapping is totally ignored (e.g., url =xyzresults in a base URL ofxyz, regardless of any other settings likemapping`)
  2. mapping specifies the base URL for assets, but has a slash, /, prepended & appended (e.g., mapping = 'xyz' results in a base URL of /xyz/, but only if url is not set)
  3. if neither url nor mapping are supplied, /assets/ is used as the base URL

If the above base URL algorithm is correct, then I will follow it for rewriting asset:-prefixed absolute URLs that don't include a hostname.

If it is incorrect, please let me know what is wrong so that I can adjust the code.

Thanks.

@davydotcom
Copy link
Contributor

Your assumptions are correct. It is also important to note a url property can be a Closure.. that takes a single argument ... the Request object.

@rgoldberg
Copy link
Contributor Author

I revamped the existing relative URL rewriting code to streamline it.

I'll try to finish testing it sometime this upcoming weekend (busy with work until then).

I also wrote some code that starts to implement the asset: scheme for absolute URL rewriting.

I'll wait until the relative URL rewrite code is finished & tested, however, before continuing with the asset: scheme, since asset: is based on the revamped relative URL code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants