I committed a change the at least deals with the default case by adding the
container to the URI in the correct places and fixing a strict check for
when a container proxy host value is not present.

To help people upgrading I'd suggest being lenient and retaining the
previous behavior by default.

Furthermore unless we add a signature to the URLs it seems like these
host/container checks are only of limited usefulness.  ie:

  var foo = gadgets.io.getProxyUrl("http://foo.com";)
    .replace("container=xyz","container=abc")
    .replace("hostname", "otherhost");


On Mon, Mar 22, 2010 at 3:42 PM, John Hjelmstad <[email protected]> wrote:

> Hi Paul:
>
> I hear you on the concern here. We're going from very distributed,
> loose-form URI generation to something more structured. My original CLs for
> DefaultProxyUriManager took this even too far, validating host and path
> prefix, which caused quite a few breakages that were unnecessary.
>
> In this case it sounds like the problem is that the old style didn't
> require
> container=. In DefaultProxyUriManager I mandate this as well for auditing
> purposes, doing so under the assumption that the URL is generated by the
> same Manager (or already-compatible gadgets.io.getProxyUrl).
> ContentRewriterUris doesn't include this in the fallback case (no container
> specified).
>
> So:
>
> 1. The intent is to get rid of HTMLContentRewriter and its entire stack
> (LinkRewriter, ContentRewriterUris, et al.). They're kept around in an
> effort to let their replacements bake in and get some real-world exposure.
> For instance, we're using them in the Google instance of Shindig.
> 2. Tactically speaking it sounds like ContentRewriterUris should tack on
> container=default (or similar) when no container value is specified, to
> support dual-mode URI generation. In this case, lenient-mode parsing should
> work fine.
>
> Thoughts?
> --j
>
> On Fri, Mar 19, 2010 at 6:11 PM, Paul Lindner <[email protected]>
> wrote:
>
> > Hi,
> >
> > Looks like proxy rewriting of images is failing with the new
> > DefaultProxyUriManager class.
> >
> > Specifically rewriters are not setting the container value, which causes
> > the
> > request to be rejected.  The default code that uses values from
> > java/config/shindig.properties and the javascript code for getProxyUrl do
> > not set container values, thus proxying is all broken for these use
> cases.
> >
> > It appears that ContentRewriterUris tries to use the container specific
> > value of gadgets.rewriteProxyBase, failing that it uses the injected
> value
> > of shindig.content-rewrite.proxy-url which maps to /gadgets/proxy?url=
> >
> > I'm a bit concerned that there are other issues like this lurking
> about...
> >
> > Is it possible that we can come up with a common idiom that handles all
> of
> > these situations?  Perhaps a parameterized Uri concept that could be
> > applied
> > here.
> >
>

Reply via email to