On Sun, Apr 8, 2012 at 12:21 PM, Thomas Mortagne <[email protected]> wrote: > On Sun, Apr 8, 2012 at 4:00 AM, Sergiu Dumitriu <[email protected]> wrote: >> On 04/06/2012 04:57 AM, Thomas Mortagne 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) >>> * 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 ? >> >> >> I don't really see the point. We should just leave them relative, adding yet >> another getURL method will only bloat the API even more. > > Yes we can forget about the public API for now, it's not very > important. As for the configuration I see it more as a > retro-compatibility security, sometimes you can't or don't know how to > properly setup you application server or httpd and setup xwiki.home > and things like that. It's important to have the right behavior by > default (relative) but since it does not cost much to add a if in > XWikiDocument#getURL and that it's enough to go back to previous > behavior for anyone that need to it's probably safer.
Here is what I plan to introduce: #-# [Since 4.0RC1] #-# Indicate if the URL used in HTTPSevlet#sendRedirect should be made absolute by XWiki or left to application server. #-# Sending absolute URLs is a bad practice and generally not needed. This option is mostly here as retro-compatibility #-# switch and you should always make sure to properly configure your application server or any proxy behind it before #-# using this. #-# 0: send relative URLs (the default) #-# 1: send absolute URLs # xwiki.redirect.absoluteurl=0 > >> >> Perhaps I wasn't making myself clear, I'm not against using relative URLs in >> sendRedirect, I'm saying that the fact that we switch to relative URLs >> doesn't really solve the problem, which is that XWiki doesn't know the >> external host and protocol, it will just hide it into less frequently used >> features, like PDF export, which makes it harder to spot the fact that the >> configuration isn't correct. >> >> >>> 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
