Problems with proxying code

[Paul Lindner <pl...@linkedin.com>]

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.

Re: Problems with proxying code

[John Hjelmstad <fa...@google.com>]

Thanks Paul. I agree w/ you -- something tells me lenient-mode will be
always-mode. It was an ill-considered added feature ;)

--j

On Mon, Mar 22, 2010 at 4:19 PM, Paul Lindner <li...@inuus.com> wrote:

> 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 <fa...@google.com> 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 <pl...@linkedin.com>
> > 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.
> > >
> >
>

Re: Problems with proxying code

[Paul Lindner <li...@inuus.com>]

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 <fa...@google.com> 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 <pl...@linkedin.com>
> 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.
> >
>

Re: Problems with proxying code

[John Hjelmstad <fa...@google.com>]

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 <pl...@linkedin.com> 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.
>