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(). > > 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 _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
