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 <[email protected]> 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 <[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. > > > > > >
