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. > >> >> 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
