Community
Participate
Working Groups
Here's an example of what happens: 1) I try to fetch http://cvs.rm.sise/arendusweb 2) Get's redirected by the Apache web server to http://cvs.rm.sise/arendusweb/ (slash appended) 3) Since Web connector checks that if the URL doesn't being with "/" then it's appended (WebRepositoryConnector.java:433), the final URL is going to be invalid: /http://cvs.rm.sise/arendusweb/ Actually, I'm not sure when it would be valid to add the "/". Is there a specific reason/use case for doing that? Please please please fix for 1.0! :) I see the solution as removing lines 433-435.
I think that slash added by the patch Willian made for bug 165852. To investigate this I either need access to the web site you have issues with, or an output from the wget -d when fetching this web site. On windows you can run it like: wget -d "_site_url_" > site.log 2>&1
Created attachment 55338 [details] wget log It's an intranet site. I'll attach the site.log file you wanted.
Erkki, is that a Codelogic installation? I wonder if you have tried to use template for Codelogic we have in web connector? I looked at the log you attached, and it seems like you may be able to avoid these redirects if you'll use final url http://cvs.rm.sise/arendusweb/index.php?cookietest=1 We'll look at improving general error detection and redirection logic and after 1.0. As always, patches and test cases are very appreciated. :-)
I think you meant Changelogic then yes, it is. And yes, I used your template, not my own. Also, your fix works fine, thanks a lot! Why didn't I think of that, my brain must have short-circuited somewhere :) I'll try to provide a test case / patch when I have more time. Haven't practically even looked at how exactly the testing is done in Eclipse projects.
(In reply to comment #4) > I'll try to provide a test case / patch when I have more time. Haven't > practically even looked at how exactly the testing is done in Eclipse projects. The only test we have right now is WebRepositoryConnectorTest, which is testing all existing templates on live servers. You can run it using Eclipse's JUnit plugin test launch configuration.
According to the http specification: http://tools.ietf.org/html/rfc2616#section-14.30 The Location header returns a URI, so we cannot we assumption if it will return a full http URL, or just a relative URL. I made some tests with wget trying to access protected sites that returns a Location header to the login page, and they return the URI with a "/" prepended. I don't know why, but the mantis installation I'm trying to access is return the Location header with a relative URL without the "/" prepended: "Location: login_page.php". Do any of you know if this is valid from http specification point of view? Apparently the server I'm trying to access is not respecting the rules... The server is a: "Server: Apache/1.3.37 (Unix) mod_auth_passthrough/1.8 mod_log_bytes/1.2 mod_bwlimited/1.4 PHP/4.4.3 FrontPage/5.0.2.2635.SR1.2 mod_ssl/2.8.28 OpenSSL/0.9.7a" So, apparently my changes into WebRepositoryConnector are wrong, but we need a workaround :) We need to parse the Location header, and if it is a relative URL without the "/", prepend it before doing the GET.
Created attachment 55362 [details] Patch
Created attachment 55363 [details] mylar/context/zip
The attached patch presents a very low-risk, but non-definitive workaround to this situation. It checks if the return URL does not contains a "/" AND does not start with a "http://" to prepend the "/". Eugene/Mik, can you please review to possibly fix for 1.0?
(In reply to comment #9) > The attached patch presents a very low-risk, but non-definitive workaround to > this situation. > > It checks if the return URL does not contains a "/" AND does not start with a > "http://" to prepend the "/". How about https:// and some test for Mantis ? :-) I'll look at this next week (probably on the weekend).
(In reply to comment #10) > How about https:// and some test for Mantis ? :-) > I'll look at this next week (probably on the weekend). > I was supposing you are willing to include on 1.0, so Erkki don't have to wait 2 weeks to 1.0.1. But it is ok. Regarding Mantis, I commented on bug#165852 that I created the credentials to the tests, but there are something wrong yet because I reverted my patch and the tests are still working. I didn't submit because I hadn't time yet to investigate deeply (perhaps on next week too).
(In reply to comment #11) > I was supposing you are willing to include on 1.0, so Erkki don't have to wait > 2 weeks to 1.0.1. But it is ok. The game for 1.0 is already over. Mik is not taking any changes. However Erkki have working workaround for his issue.
HttpClient is supposed to handle relative URIs. When a method with an absolute URI is executed, the URI is stored in HostConfiguration I wonder why it didn't work in your case. Some of the interesting code is in HttpMethodDirector.processRedirectResponse(HttpMethod) (beginning at line 598: if (redirectUri.isRelativeURI()) { ... } ) There is also a parameter there that is checked: REJECT_RELATIVE_REDIRECT. I suppose this is false by default though. But maybe instead of reinventing HTTP redirection we should investigate why HttpClient was throwing a CircularRedirectException? Actually, following the above REJECT_... parameter, I also noticed: HttpClientParams.ALLOW_CIRCULAR_REDIRECTS . Maybe we can just set this parameter somehow and let HttpClient handle the redirects.
I agree. We should investigate what is with that circular redirect error.
Just to be clear: we had to stop making code changes of this sort early on Friday, and focus entirely on testing, because at this stage the cost of destabilizing is so much higher than the cost of improving or fixing any particular feature. This can still go into next Friday's dev build if Eugene is happy with the patch by that point.
Tentatively scheduling for 1.0.1.
Eugene: are you planning on having this for 1.0.1? If so we should get it into a dev build by Wednesday so that others can try it out.
I'll try to look at these tonight..
Created attachment 56024 [details] removing all custom redirect logic Erkki, Willian, can you try if this patch works for your repositories? I removed all redirect logic. WebRepositoryConnector test is passing. I also tried some mantis and changelogic repositories and they are working too...
Created attachment 56025 [details] mylar/context/zip
Patch applied. Commented out statusCode local var because it was never read.
(In reply to comment #21) > Patch applied. Commented out statusCode local var because it was never read. Ehhhhgm... I know it was exiting for you to apply using that new "apply patch" feature in bugzilla editor, but I was hoping that guys can try it first. :-) Anyways, lets see if it will break anything and anyone will scream about it.
Mik, I just spotted that you commented out too much. Status code is not used, but client.executeMethod(method) is still should be there. I committed fix to CVS.
(In reply to comment #19) > Erkki, Willian, can you try if this patch works for your repositories? I > removed all redirect logic. WebRepositoryConnector test is passing. I also > tried some mantis and changelogic repositories and they are working too... I'm getting a CircularRedirectException with the same Changelogic server mentioned above (with the "/index.php?cookietest=1" appended to the login form url). The problem is that it's redirecting to the same file index.php, and parameters are not taken into account when deciding whether it's a circular redirect. The place where the exception gets thrown is HttpMethodDirector.java:633 Personally, I think that since browsers are allowing circular redirects, so should Mylar. Also, I don't see anything about disallowing this in HTTP/1.1 spec. Maybe it's a question of interpretation. Doing that would mean setting the parameter HttpClientParams.ALLOW_CIRCULAR_REDIRECTS to true. (Max number of redirects will still be in effect, default seems to be 100). I would put this to WebClientUtil, see attached patch.
Created attachment 56076 [details] Allow Circular Redirects
Created attachment 56077 [details] mylar/context/zip
Heh. Is it reproduceable on any public repository?
Yes, actually I'm get the same results with the Aranea framework repository (while logging in).
(In reply to comment #19) > Erkki, Willian, can you try if this patch works for your repositories? I > removed all redirect logic. WebRepositoryConnector test is passing. I also > tried some mantis and changelogic repositories and they are working too... > Is there a dev-build I can try?
Also, the WebRepositoryConnectorTest was failing for me for the Aranea Framework (Changelogic) template before this patch and passes with this patch. What could be the difference in our configuration?
(In reply to comment #29) > Is there a dev-build I can try? Forget about it. I just tested with HEAD+Erkki latest patch at home and it works. But there is a issue yet: the query returns results, but if you open any of them, the browser editor opens on the login screen.
That should be normal, because the cookies received after login are only used by HttpClient for the query, they are not passed to Eclipse's internal browser. I don't know if it's even possible to do that. So the internal browser is not logged in. But it shouldn't matter much because any sessions in the internal browser should last accross Eclipse sessions (I guess the cookies are stored somewhere in the workspaces .metadata folder?) -- so you only have to login once.
Erkki's redirect patch looked good to me, so applied and Aranea test passes. Eugue: please take a look over this to see if there is anything remaining.
