REPROPOSAL : Content rewriter feature

[Louis Ryan]

Re-proposing the content rewriter feature for 0.9 that did'nt quite make it for 0.8.1.

Original thread is here: https://groups.google.com/group/opensocial-and-gadgets-spec/browse_thread/thread/7a9bd86e74ab803e/5448bfb5084c373e?lnk=gst&q=rewriter#5448bfb5084c373e

Changes from the original thread

- removal of include-tags as its essentially redundant with the ability to match by links in tags anyway and was causing confusion in practice as people were trying to use regexs here.

-------------------------------------------
Proposal

Add support for rewriting the content of a generated gadget and allow developers to control how the behavior of the rewriter by introducing a new standard optional gadget feature content-rewrite.

E.g.
<Optional feature="content-rewrite">
        <Param name="expires">86400</Param>

        <Param name="include-urls">.*</Param>

        <Param name="exclude-urls"></Param>
        <Param name="minify-css">true</Param>
        <Param name="minify-js">true</Param>
        <Param name="minify-html">true</Param>

</Optional>

The feature supports the following parameters:

expires - The duration in seconds to force as minimum HTTP cache time for content fetched through the proxy via a rewritten URL. Default 86400

include-urls - A regex used to match URLs to rewrite to proxied form. Default .*

exclude-urls - A regex used to match URLs to exclude from rewriting. Processed after include-urls. Default is not to exclude any URL if not specifed

minify-css - Controls whether the container will attempt to minify css in style tags and referenced css files. Valid values are true|false. Container specific default

minify-js - Controls whether the container will attempt to minify JS in script tags and referenced JS files. Valid values are true|false. Container specific default

minify-html - Controls whether the container will attempt to minify HTML content. Valid values are true|false. Container specific default


Containers are free to perform additional optimizations when rewriting links including but not limited to:

- Extract @import directives from style tags and convert them into link tags in the head tag
- Merge multiple CSS fetches from successive link tags into one link tag that causes the proxy to concatenate the content fetched from the individual URLs
- Merge contiguous <srcipt src=xxx> tags into one concatenating proxy fetch

[John Hjelmstad]

On Wed, Oct 29, 2008 at 3:28 PM, Louis Ryan <lr...@google.com> wrote:

Re-proposing the content rewriter feature for 0.9 that did'nt quite make it for 0.8.1.

Original thread is here: https://groups.google.com/group/opensocial-and-gadgets-spec/browse_thread/thread/7a9bd86e74ab803e/5448bfb5084c373e?lnk=gst&q=rewriter#5448bfb5084c373e

Changes from the original thread

- removal of include-tags as its essentially redundant with the ability to match by links in tags anyway and was causing confusion in practice as people were trying to use regexs here.

-------------------------------------------
Proposal

Add support for rewriting the content of a generated gadget and allow developers to control how the behavior of the rewriter by introducing a new standard optional gadget feature content-rewrite.

E.g.
<Optional feature="content-rewrite">
        <Param name="expires">86400</Param>

        <Param name="include-urls">.*</Param>

        <Param name="exclude-urls"></Param>
        <Param name="minify-css">true</Param>
        <Param name="minify-js">true</Param>
        <Param name="minify-html">true</Param>

</Optional>

The feature supports the following parameters:

expires - The duration in seconds to force as minimum HTTP cache time for content fetched through the proxy via a rewritten URL. Default 86400

include-urls - A regex used to match URLs to rewrite to proxied form. Default .*

exclude-urls - A regex used to match URLs to exclude from rewriting. Processed after include-urls. Default is not to exclude any URL if not specifed

minify-css - Controls whether the container will attempt to minify css in style tags and referenced css files. Valid values are true|false. Container specific default

minify-js - Controls whether the container will attempt to minify JS in script tags and referenced JS files. Valid values are true|false. Container specific default

minify-html - Controls whether the container will attempt to minify HTML content. Valid values are true|false. Container specific default

What's the rationale for including functionally neutral - that is, having neither impact on the app's functionality nor the developer's operations (proxying seems reasonable for this)?

I'd argue that minification that breaks a gadget is simply buggy, so should be fixed/not turned on to begin with. For debugging purposes, &debug=1 should always be honored, and should probably disable all rewriting.

 

Containers are free to perform additional optimizations when rewriting links including but not limited to:

- Extract @import directives from style tags and convert them into link tags in the head tag
- Merge multiple CSS fetches from successive link tags into one link tag that causes the proxy to concatenate the content fetched from the individual URLs
- Merge contiguous <srcipt src=xxx> tags into one concatenating proxy fetch

Should we suggest a convention for <Param>s that configure alternate rewriting behaviors as well?

--John

 

[Louis Ryan]

While I agree in principal I think that there will very likely be outliers for a couple of reasons. 

- Minifier has a bug that affects a very limited content structure but works just fine for 99%+ of devs. We'd like to keep it enabled while we work on the bug. An opt-out lets us work through this.

- People think their content is already minified enough and find ours annoying for readability purposes. If the container default is on they  would want to explicitly turn it off.

 

 

Containers are free to perform additional optimizations when rewriting links including but not limited to:

- Extract @import directives from style tags and convert them into link tags in the head tag
- Merge multiple CSS fetches from successive link tags into one link tag that causes the proxy to concatenate the content fetched from the individual URLs
- Merge contiguous <srcipt src=xxx> tags into one concatenating proxy fetch

Should we suggest a convention for <Param>s that configure alternate rewriting behaviors as well?

Im not sure if we need to yet. I want proposing that those additional optimizations be controlled by new params but rather that they should be considered as natural consequences of link rewriting in general.

 

--John

 

[John Hjelmstad]

That sounds like a reasonable commonsense argument to me in whatever context it applies. Is this just for JS, or HTML/CSS as well?

[Louis Ryan]

I think its safer to assume its for everything.

[Evan Gilbert]

Can we add the requirement that containers provide a way to disable rewriting on a per-gadget render basis (probably using a URL param but I could also see persistent cookies for developers)?

[Louis Ryan]

Yes, I would be fine with rewrite = on | off | force where on -> use spec in gadget, off -> disable & force -> ask container to enable even if not on by default in container.

[Brian Eaton]

On Wed, Oct 29, 2008 at 2:28 PM, Louis Ryan <lr...@google.com> wrote:
> include-urls - A regex used to match URLs to rewrite to proxied form.
> Default .*
>
> exclude-urls - A regex used to match URLs to exclude from rewriting.
> Processed after include-urls. Default is not to exclude any URL if not
> specifed

We should not support execution of user supplied regular expressions.
This creates a significant risk of DOS or worse. Would prefix
matching provide sufficient functionality?

Even perfect regular expression libraries have exponential worst-case run times.

Buggy regular expression libraries allow execution of arbitrary code.
For example:

http://www.securityfocus.com/bid/26346
http://www.cisco.com/en/US/products/products_security_response09186a00808bb91c.html
http://www.linuxdevcenter.com/pub/a/linux/2003/12/29/insecurities.html

[Kevin Brown]

On Mon, Nov 3, 2008 at 11:35 AM, Brian Eaton <bea...@google.com> wrote:

On Wed, Oct 29, 2008 at 2:28 PM, Louis Ryan <lr...@google.com> wrote:

> include-urls - A regex used to match URLs to rewrite to proxied form.
> Default .*
>
> exclude-urls - A regex used to match URLs to exclude from rewriting.
> Processed after include-urls. Default is not to exclude any URL if not
> specifed

We should not support execution of user supplied regular expressions.
This creates a significant risk of DOS or worse.  Would prefix
matching provide sufficient functionality?

A list of simplified glob patterns (only allow * and ? wild cards) would be preferred. This would allow for:

<Param name="include-urls">http://example.org/*.css, *.png, *.gif, *.jpg</Param>
 

[John Hjelmstad]

+1 to this modification.

[Brian Eaton]

On Mon, Nov 3, 2008 at 12:52 PM, Kevin Brown <et...@google.com> wrote:

> A list of simplified glob patterns (only allow * and ? wild cards) would be
> preferred. This would allow for:
>
> <Param name="include-urls">http://example.org/*.css, *.png, *.gif,
> *.jpg</Param>

Shell-style globbing still allows very bad worst-case runtimes, via
patterns that have multiple wild cards, e.g. *a*a*.

If we restrict to a single "*" wild card we can make the matching time
linear in the length of the string to be tested. That still permits
the patterns you gave above. (Implementation is also trivial, just
split on '*' and then do a prefix and suffix match. A pattern with no
* is a special case that is evaluated via string equality.)

[Louis Ryan]

Im fine with a single glob char matcher, this really does not need alot of power. If we use glob though we will need to support lists of matches, Im happy to let the include-urls and exclude-urls repeat.

[Scott Seely]

Louis-- can you post an updated patch for this spec change?

On Nov 3, 1:59 pm, Louis Ryan <lr...@google.com> wrote:
> Im fine with a single glob char matcher, this really does not need alot of
> power. If we use glob though we will need to support lists of matches, Im
> happy to let the include-urls and exclude-urls repeat.
>
> On Mon, Nov 3, 2008 at 2:29 PM, Brian Eaton <bea...@google.com> wrote:
>

[Louis Ryan]

Actually latest code for this all lives in Shindig (theres no JS)

[Louis Ryan]

Come useful pointers....

Parsing feature
http://svn.apache.org/repos/asf/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java

DOM based implementation of rewriting
http://svn.apache.org/repos/asf/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/HTMLContentRewriter.java

Note this code isnt precisely tracking every suggestion on the thread (glob vs regex) etc but it should be enough...

[Scott Seely]

And the spec would be “port this”?

[Louis Ryan]

Im many ways, yes. I believe this is 100% within the spirit of 'provide a working implementation' and many of the larger sample implementations will be coming out of Shindig e.g. proxied content for which there is an initial implementation there too. Its not really practical to upload things of this nature to the wiki because they have so many dependencies.

[Louis Ryan]

I'll move the original spec to the wiki and provide a link to the code from there. Seems best.

[Scott Seely]

Update from F2F:

After some talking, we decided that it was more important to use
substring matching instead of globbing (* character) or regex.
Substring is the simplest way to solve the pattern matching mechanism.

On Nov 11, 5:15 pm, Louis Ryan <lr...@google.com> wrote:
> I'll move the original spec to the wiki and provide a link to the code from
> there. Seems best.
>
> On Tue, Nov 11, 2008 at 5:13 PM, Louis Ryan <lr...@google.com> wrote:
> > Im many ways, yes. I believe this is 100% within the spirit of 'provide a
> > working implementation' and many of the larger sample implementations will
> > be coming out of Shindig e.g. proxied content for which there is an initial
> > implementation there too. Its not really practical to upload things of this
> > nature to the wiki because they have so many dependencies.
>
> > On Tue, Nov 11, 2008 at 4:53 PM, Scott Seely <sSe...@myspace.com> wrote:
>
> >>  And the spec would be "port this"?
>

> >> *From:* opensocial-an...@googlegroups.com [mailto:
> >> opensocial-an...@googlegroups.com] *On Behalf Of *Louis Ryan
> >> *Sent:* Tuesday, November 11, 2008 3:34 PM
> >> *To:* opensocial-an...@googlegroups.com
> >> *Subject:* [opensocial-and-gadgets-spec] Re: REPROPOSAL : Content

> >> rewriter feature
>
> >> Come useful pointers....
>
> >> Parsing feature
>

> >>http://svn.apache.org/repos/asf/incubator/shindig/trunk/java/gadgets/...

>
> >> DOM based implementation of rewriting
>

> >>http://svn.apache.org/repos/asf/incubator/shindig/trunk/java/gadgets/...

[Louis Ryan]

Proposal updated

http://wiki.opensocial.org/index.php?title=Content_Rewriter_Feature

Added details on substring matching and special use of literal "*" to represent match all

[Scott Seely]

Potential usability issue: since ‘include-urls’ and ‘exclude-urls’ can really only include one URL per entry, it will sense to make the term singular (include-url  exclude-url). Reason: when scanning this, I first assumed comma-delimited list for the param. After actually reading this, I learned my scan was wrong.

[Louis Ryan]

Good point, updating....

[Scott Seely]

One final issue: the include-url/exclude-url logic assumes case-sensitive matching. This is hard to get right and many developers wouldn’t know they got it wrong. This is an issue that hits experienced and inexperienced alike.

 

Suggestion: make case-insensitive matches the default. Update the feature to look like this:

 

<Optional feature="content-rewrite">

       <Param name="case-sensitive-exclude-url">true</Param>

       <!-- Exclude .GiF only ... -->

       <Param name="exclude-url">.GiF</Param>

</Optional>

 

Possible alternative

 

<Optional feature="content-rewrite">

       <!-- Exclude .GiF only ... -->

       <Param name="exclude-url" caseSensitive="true">.GiF</Param>

</Optional>

[John Hjelmstad]

I favor the former, or something like:

<Param name="exclude-urls">.gIF::case-insensitive</Param>

...though this suggests there are certain character groupings (eg. ::) that we would disallow in a substring match.

The attribute suggestion (caseInsensitive="true") extends the gadget XSD, and adds additional semantics atop the N/V <Param> syntax. Doing that is valuable, but I'd prefer it be done in a more generic way at some later point.

John

[Kevin Brown]

Is there actually a good reason not to just use case insensitive matching in all cases? I can think of plenty of contrived reasons for wanting case sensitivity, but for this it seems unlikely.

[Louis Ryan]

I dont think we need to overspecify here, case-insensitive matching as the standard behavior works fine for me. If people need case sensitive descrimination later we can always add a casesensitive="true" attribute in a later release. Ill update the spec to reflect this.

[Scott Seely]

The only cases, non-contrived, that I am aware of involve Unix deployments where /images and /Images are valid, side by side directory names. In other words, the non-contrived case is: Developer has a poorly laid out directory structure and needs to use the feature.

 

Given this sentiment, I’m happy with John Hjelmstad’s suggestion to make case sensitivity a global switch where it is either on or off and, by default, it is off.

 

The options are then

 

<Optional feature="content-rewrite">

       <Param name="case-sensitive-exclude-url">true</Param>

       <Param name="case-sensitive-include-url">true</Param>

       <!-- Exclude .GiF only ... -->

       <Param name="exclude-url">.GiF</Param>

</Optional>

Or

<Optional feature="content-rewrite">

       <!-- One switch to enable/disable case-sensitivity on URLs -->

       <Param name="case-sensitive-url">true</Param>

       <!-- Exclude .GiF only ... -->

       <Param name="exclude-url">.GiF</Param>

</Optional>

I’m in favor of fewer switches, so I’d prefer the last option. That said, if someone says “I have a use case that shows we really need to handle this on both”, I’ll happily accept your use case. To me, case-insensitive comparison by default is more important than a discussion about whether to have 1 or 2 switches.

[Scott Seely]

+1

[John Hjelmstad]

+1

[Scott Seely]

After some discussion internally, it sounds like EXPIRES may be problematic in this iteration. Does anyone object to making EXPIRES optional in the spec with behavior to ignore the parameter when it’s not supported?

[Louis Ryan]

What is the specific issue you have with expires? Im not strongly opposed to making it optional just curious.

[Bryan Green]

As we refresh apps, and store content, we serve up new URLs, even if the developers use same URLs in their XML.  So, the caching/refreshing happens automatically.  Developers won’t need to worry about expiration timing. 

 

I’m +1

[Bess Ho]

+1

This is a better implementation. Suggest default is  case insensitive

<Optional feature="content-rewrite">

       <!-- Exclude .GiF only ... -->

       <Param name="exclude-url" caseSensitive="true">.GiF</Param>

</Optional>

--
Bess Ho

[Scott Seely]

passed

On Nov 19, 8:34 pm, "Bess Ho" <bess...@gmail.com> wrote:
> +1
>
> This is a better implementation. Suggest default is  case insensitive
>
> <Optional feature="content-rewrite">
>
>        <!-- Exclude .GiF only ... -->
>
>        <Param name="exclude-url" caseSensitive="true">.GiF</Param>
>
> </Optional>
>
>
>
> On Wed, Nov 19, 2008 at 5:39 PM, Bryan Green <bgr...@myspace.com> wrote:
> >  As we refresh apps, and store content, we serve up new URLs, even if the
> > developers use same URLs in their XML.  So, the caching/refreshing happens
> > automatically.  Developers won't need to worry about expiration timing.
>
> > I'm +1
>

> > *From:* opensocial-an...@googlegroups.com [mailto:
> > opensocial-an...@googlegroups.com] *On Behalf Of *Louis Ryan

> > *Sent:* Wednesday, November 19, 2008 5:25 PM

>
> > *To:* opensocial-an...@googlegroups.com
> > *Subject:* [opensocial-and-gadgets-spec] Re: REPROPOSAL : Content rewriter
> > feature
>

> > What is the specific issue you have with expires? Im not strongly opposed
> > to making it optional just curious.
>
> > On Wed, Nov 19, 2008 at 4:19 PM, Scott Seely <sSe...@myspace.com> wrote:
>
> > After some discussion internally, it sounds like EXPIRES may be problematic
> > in this iteration. Does anyone object to making EXPIRES optional in the spec
> > with behavior to ignore the parameter when it's not supported?
>

> > *From:* opensocial-an...@googlegroups.com [mailto:
> > opensocial-an...@googlegroups.com] *On Behalf Of *John
> > Hjelmstad
> > *Sent:* Wednesday, November 19, 2008 2:45 PM

>
> > *To:* opensocial-an...@googlegroups.com
> > *Subject:* [opensocial-and-gadgets-spec] Re: REPROPOSAL : Content rewriter
> > feature
>

> > +1
>
> > On Wed, Nov 19, 2008 at 2:34 PM, Scott Seely <sSe...@myspace.com> wrote:
>
> > +1
>

> > *From:* opensocial-an...@googlegroups.com [mailto:
> > opensocial-an...@googlegroups.com] *On Behalf Of *Louis Ryan

> > *Sent:* Wednesday, November 19, 2008 1:46 PM

>
> > *To:* opensocial-an...@googlegroups.com
> > *Subject:* [opensocial-and-gadgets-spec] Re: REPROPOSAL : Content rewriter
> > feature
>

> > I dont think we need to overspecify here, case-insensitive matching as the
> > standard behavior works fine for me. If people need case sensitive
> > descrimination later we can always add a casesensitive="true" attribute in a
> > later release. Ill update the spec to reflect this.
>

> > On Wed, Nov 19, 2008 at 1:40 PM, Kevin Brown <e...@google.com> wrote:
>
> > Is there actually a good reason not to just use case insensitive matching
> > in all cases? I can think of plenty of contrived reasons for wanting case
> > sensitivity, but for this it seems unlikely.
>
> > On Wed, Nov 19, 2008 at 12:04 PM, Scott Seely <sSe...@myspace.com> wrote:
>
> > One final issue: the include-url/exclude-url logic assumes case-sensitive
> > matching. This is hard to get right and many developers wouldn't know they
> > got it wrong. This is an issue that hits experienced and inexperienced
> > alike.
>
> > Suggestion: make case-insensitive matches the default. Update the feature
> > to look like this:
>
> > <Optional feature="content-rewrite">
>
> >        <Param name="case-sensitive-exclude-url">true</Param>
>
> >        <!-- Exclude .GiF only ... -->
>
> >        <Param name="exclude-url">.GiF</Param>
>
> > </Optional>
>
> > Possible alternative
>
> > <Optional feature="content-rewrite">
>
> >        <!-- Exclude .GiF only ... -->
>
> >        <Param name="exclude-url" caseSensitive="true">.GiF</Param>
>
> > </Optional>
>

> > *From:* opensocial-an...@googlegroups.com [mailto:
> > opensocial-an...@googlegroups.com] *On Behalf Of *Louis Ryan

> > *Sent:* Tuesday, November 18, 2008 10:16 AM

>
> > *To:* opensocial-an...@googlegroups.com
> > *Subject:* [opensocial-and-gadgets-spec] Re: REPROPOSAL : Content rewriter
> > feature
>

> > Good point, updating....
>
> > On Tue, Nov 18, 2008 at 10:09 AM, Scott Seely <sSe...@myspace.com> wrote:
>
> > Potential usability issue: since 'include-urls' and 'exclude-urls' can
> > really only include one URL per entry, it will sense to make the term
> > singular (include-url  exclude-url). Reason: when scanning this, I first
> > assumed comma-delimited list for the param. After actually reading this, I
> > learned my scan was wrong.
>

> > *From:* opensocial-an...@googlegroups.com [mailto:
> > opensocial-an...@googlegroups.com] *On Behalf Of *Louis Ryan

> > *Sent:* Tuesday, November 18, 2008 12:04 AM

> > > >>http://svn.apache.org/repos/asf/incubator/shindig/trunk/java/gadgets/.<http://svn.apache.org/repos/asf/incubator/shindig/trunk/java/gadgets/>
> > ..

>
> > > >> DOM based implementation of rewriting
>

> > > >>http://svn.apache.org/repos/asf/incubator/shindig/trunk/java/gadgets/.<http://svn.apache.org/repos/asf/incubator/shindig/trunk/java/gadgets/>
> > ..

[Eiji Kitamura]

Hi,


I see content rewrite feature's spec proposal here:
http://wiki.opensocial.org/index.php?title=Content_Rewriter_Feature

and draft spec for whole gadget spec here:
http://sites.google.com/site/opensocialdraft/Home/gadgets-api-specification

they both says including or excluding urls works by specifying param
name as "include-url" or "exclude-url", while discussion on this
thread says they should be "include-urls" or "exclude-urls".

I believe "include-urls" or "exclude-urls" are valid spec and
"include-url" or "exclude-url" are typo. Is that correct?


If so, can someone please correct documents? It's confusing.

2008/11/25 Scott Seely <sse...@myspace.com>: