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
