Re: [xwiki-devs] [PROPOSAL] Use relative URLs instead of external URLs with sendRedirect

On Sun, Apr 8, 2012 at 1:15 PM, Thomas Mortagne <thomas.mortagne(a)xwiki.com> wrote: > On Fri, Apr 6, 2012 at 11:28 AM, Marius Dumitru Florea > <mariusdumitru.florea(a)xwiki.com> wrote: >> On Fri, Apr 6, 2012 at 11:57 AM, Thomas Mortagne >> <thomas.mortagne(a)xwiki.com> wrote: >>> I was starting to look at what to modify and I found that we are >>> really not consistent right now: most of the Java code in oldcore like >>> XWikiAction are using absolute URL but all VM I could see (and I guess >>> it's the same for the wiki pages) send relative URLs. >>> >>> So using absolute URL for send redirect is very far from being a rule >>> right now. It also mean that making it configurable is going to be a >>> pain since none of the .vm and wiki page are going though some common >>> redirect oriented URL generator like most Java code do. >>> >>> Here is a proposal: >>> * introduce a configuration but use it only in >>> XWikiDocument#getURL(String action, String params, boolean redirect, >>> XWikiContext context) (that's the method where pretty much all Java >>> code go trough when asking for a redirect URL) >> >> There is also api.XWiki.getRequestURL() used with xredirect query >> string parameter, at least for the login and logout URLs. We should >> either add an API to get the relative form of the current request URL >> or change api.XWiki.getRequestURL() to take into account the >> configuration you're proposing. I prefer the later. > > I don't agree here, api.XWiki.getRequestURL() is supposed to return > the request URL and returning a relative URL would not make much sense > and would be a major API breackage IMO. If you return a relative URL > it's not really the request URL anymore. > > Now I'm not sure it's that important to force this particular use case > in a relative URL since what you want is to go back in the exact same > situation after having done something (like login) and providing the > request URL as it is probably make more sense that modify it in a > relative URL to be resolved back by the application server. > > WDYT ? We don't have the "request URL as it is". We compute it using the same URL factory that creates wrong document external URLs when XWiki is behind a proxy. So if XWikiDocument#getURL() returns the wrong URL then XWiki#getRequestURL will do the same. I'm fine with keeping the current behaviour of XWiki#getRequestURL() for backward compatibility, but we need an API to get the relative request URL as well.

Thanks, Marius > >> >>> * introduce a api.Document#getURL(String action, String queryString, >>> boolean redirect) method to follow XWikiDocument (from scratch I think >>> I would prefer getRedirectURL but it's simpler to follow already >>> existing API for now) >>> * we can refactor later the .vm and wiki page to support this new >>> configuration. What's important right now is to allow someone to come >>> back to old behavior for whatever reason (which sounds a lot less >>> necessary to me now that I know that we actually use a lot of relative >>> URL in sendRedirect) but better be safe since it does not cost a lot >>> here and I plan to introduce this in xwiki.cfg only >>> >>> WDYT ? >> >> +1, although I'm also for generating relative URLs whenever it is >> possible, without any configuration parameter. >> >> Thanks, >> Marius >> >>> >>> On Wed, Mar 21, 2012 at 1:10 AM, Thomas Mortagne >>> <thomas.mortagne(a)xwiki.com> wrote: >>>> On Tue, Mar 20, 2012 at 11:36 PM, Sergiu Dumitriu <sergiu(a)xwiki.com> wrote: >>>>> On 03/20/2012 03:39 AM, Thomas Mortagne wrote: >>>>>> >>>>>> Hi devs, >>>>>> >>>>>> In HTTP specifications a redirect is always absolute URL which is >>>>>> probably why we use absolute URL with sendRedirect. >>>>>> >>>>>> However sendRedirect does not produce direct HTTP response but allows >>>>>> relative URL and delegate to the application server the job of >>>>>> producing proper absolute URL. >>>>>> >>>>>> IMO XWiki should always use relative URL everywhere it can so I >>>>>> propose to change our practice to use relative URL instead of absolute >>>>>> URL with HttpSevletResponse#sendRedirect when possible. >>>>>> >>>>>> The only reasons I see to use external URLs are: >>>>>> * interwiki URL in a  domain based multiwiki >>>>>> * html/pdf export for links pointing on not exported pages or non view >>>>>> actions >>>>>> >>>>>> WDYT ? >>>>> >>>>> >>>>> I don't think this will actually solve the problem. >>>> >>>> What problem ? If you are talking about >>>> http://jira.xwiki.org/browse/XWIKI-7632 it did fixed the issue in this >>>> specific use case as I said on the issue itself. >>>> >>>>>As long as XWiki doesn't >>>>> know the correct URL to use, I doubt that the container will do any better. >>>>> I just tested this on Apache HTTPD + mod_proxy_http going to Jetty, and it >>>>> didn't solve the problem. >>>> >>>> Probably mean you did not properly configured your reverse proxy but >>>> in my use case it was done right. >>>> >>>>> >>>>> For the PDF export, all URLs must be external. A relative URL in a PDF >>>>> doesn't have a base URL to work with, since the PDF is a standalone >>>>> document. That's why we use a special URLFactory when exporting PDFs. >>>> >>>> I know we are using a special URLFactory for pdf export. If a document >>>> pointing to itself or to another document exported in the same pdf is >>>> an external URL with sheme/host/port then there is something pretty >>>> wrong in the pdf export. Anyway that's not really the subject of the >>>> proposal. >>>> >>>>> >>>>> >>>>>> Here is my +1. We very often fix bugs in the way to produce external >>>>>> URL and it's still not OK (see >>>>>> http://jira.xwiki.org/browse/XWIKI-7632) so lets reduce the scope for >>>>>> this need as much as possible. >>>>>> >>>>> >>>>> >>>>> -- >>>>> Sergiu Dumitriu >>>>> http://purl.org/net/sergiu/ >>>>> >>>>> _______________________________________________ >>>>> devs mailing list >>>>> devs(a)xwiki.org >>>>> http://lists.xwiki.org/mailman/listinfo/devs >>>> >>>> >>>> >>>> -- >>>> Thomas Mortagne >>> >>> >>> >>> -- >>> Thomas Mortagne >>> _______________________________________________ >>> devs mailing list >>> devs(a)xwiki.org >>> http://lists.xwiki.org/mailman/listinfo/devs >> _______________________________________________ >> devs mailing list >> devs(a)xwiki.org >> http://lists.xwiki.org/mailman/listinfo/devs > > > > -- > Thomas Mortagne > _______________________________________________ > devs mailing list > devs(a)xwiki.org > http://lists.xwiki.org/mailman/listinfo/devs _______________________________________________ devs mailing list devs(a)xwiki.org http://lists.xwiki.org/mailman/listinfo/devs