Done, hope I did not break anything in the process. On Thu, May 3, 2012 at 2:34 PM, Marius Dumitru Florea <[email protected]> wrote: > On Thu, May 3, 2012 at 2:24 PM, Thomas Mortagne > <[email protected]> wrote: >> On Tue, Apr 10, 2012 at 10:16 AM, Thomas Mortagne >> <[email protected]> wrote: >>> On Mon, Apr 9, 2012 at 9:26 AM, Marius Dumitru Florea >>> <[email protected]> wrote: >>>> On Sun, Apr 8, 2012 at 1:15 PM, Thomas Mortagne >>>> <[email protected]> wrote: >>>>> On Fri, Apr 6, 2012 at 11:28 AM, Marius Dumitru Florea >>>>> <[email protected]> wrote: >>>>>> On Fri, Apr 6, 2012 at 11:57 AM, Thomas Mortagne >>>>>> <[email protected]> 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. >>> >>> Indeed I fault this was simply provided by the requuest since there >>> was getRequestURL in HttpServletRequest but it's reconstructed too >>> according to the javadoc. In that case yes we should probably >>> introduce a new API for it. But we need to be carefull with that in >>> the case where xredirect is using in another wiki but I don't think we >>> really have this usse case. >>> >>> What about adding a api.XWiki#getRequestURL(boolean redirect) to >>> follow XWikiDocument#getURL ? >>> >>> Other idea beiing to a simple api.XWiki#getRelativeRequestURL(). >> > >> Adding api.XWiki#getRelativeRequestURL() and I should be ready to >> merge my branch. > > Great! Thanks, > Marius > >> >>> >>>> >>>> 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 >>>>>>> <[email protected]> wrote: >>>>>>>> On Tue, Mar 20, 2012 at 11:36 PM, Sergiu Dumitriu <[email protected]> >>>>>>>> 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 >>>>>>>>> [email protected] >>>>>>>>> http://lists.xwiki.org/mailman/listinfo/devs >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Thomas Mortagne >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Thomas Mortagne >>>>>>> _______________________________________________ >>>>>>> devs mailing list >>>>>>> [email protected] >>>>>>> http://lists.xwiki.org/mailman/listinfo/devs >>>>>> _______________________________________________ >>>>>> devs mailing list >>>>>> [email protected] >>>>>> http://lists.xwiki.org/mailman/listinfo/devs >>>>> >>>>> >>>>> >>>>> -- >>>>> Thomas Mortagne >>>>> _______________________________________________ >>>>> devs mailing list >>>>> [email protected] >>>>> http://lists.xwiki.org/mailman/listinfo/devs >>>> _______________________________________________ >>>> devs mailing list >>>> [email protected] >>>> http://lists.xwiki.org/mailman/listinfo/devs >>> >>> >>> >>> -- >>> Thomas Mortagne >> >> >> >> -- >> Thomas Mortagne >> _______________________________________________ >> devs mailing list >> [email protected] >> http://lists.xwiki.org/mailman/listinfo/devs > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
