jira.codehaus.org

  • Log In Access more options
    • Online Help
    • Keyboard Shortcuts
    • About JIRA
    • JIRA Credits
    • What?s New
  • Dashboards Access more options (Alt+d)
  • Projects Access more options (Alt+p)
  • Issues Access more options (Alt+i)
  • GeoServer
  • GEOS-3410

GSIP 39: pluggable url manglers

  • Log In
  • Views
    • XML
    • Word
    • Printable

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: 2.0-RC2
  • Component/s: Global
  • Labels:
    None

Description

See http://geoserver.org/display/GEOS/GSIP+39+-+Centralized%2C+pluggable+URL+mangling

  • Options
    • Sort By Name
    • Sort By Date
    • Ascending
    • Descending
    • Download All

Attachments

  1. Text File
    GEOS-3140.patch
    09/Sep/09 10:27 AM
    128 kB
    Andrea Aime

Issue Links

is depended upon by

Improvement - An improvement or enhancement to an existing feature or task. GEOS-3411 Make sure reverse proxy filter uses RequestUtil.buildURL properly

  • Major - Major loss of function.
  • Resolved - A resolution has been taken, and it is awaiting verification by reporter. From here issues are either reopened, or are closed.

Activity

Ascending order - Click to sort in descending order
  • All
  • Comments
  • Work Log
  • History
  • Activity
Hide
Permalink
Andrea Aime added a comment - 09/Sep/09 10:30 AM

Btw, the patch is against the current trunk. It should backport more or less nicely, besides the WMS module and the rest modules that have seen some changes.

Show
Andrea Aime added a comment - 09/Sep/09 10:30 AM Btw, the patch is against the current trunk. It should backport more or less nicely, besides the WMS module and the rest modules that have seen some changes.
Hide
Permalink
Andrea Aime added a comment - 09/Sep/09 10:55 AM

One specific thing with this patch is that the url encoding of parameters is done by URLEncoder.
That turns the commas into %2C. Now, I've tried with some online url encoders and it would seem the comma and column have to be encoded:
http://www.blooberry.com/indexdot/html/topics/urlencoding.htm
http://www.permadi.com/tutorial/urlEncoding/
http://en.wikipedia.org/wiki/Percent-encoding

and also here is an online encoder:
http://www.albionresearch.com/misc/urlencode.php

However that changes the resutling url a bit (the browsers still seem happy to use them)... if that is a problem we can use a replacement such as http://www.w3.org/International/O-URL-code.html, which I did not use because of the advertisement clause in the license (but that class is short and can be rewritten quickly)

Show
Andrea Aime added a comment - 09/Sep/09 10:55 AM One specific thing with this patch is that the url encoding of parameters is done by URLEncoder. That turns the commas into %2C. Now, I've tried with some online url encoders and it would seem the comma and column have to be encoded: http://www.blooberry.com/indexdot/html/topics/urlencoding.htm http://www.permadi.com/tutorial/urlEncoding/ http://en.wikipedia.org/wiki/Percent-encoding and also here is an online encoder: http://www.albionresearch.com/misc/urlencode.php However that changes the resutling url a bit (the browsers still seem happy to use them)... if that is a problem we can use a replacement such as http://www.w3.org/International/O-URL-code.html, which I did not use because of the advertisement clause in the license (but that class is short and can be rewritten quickly)
Hide
Permalink
Justin Deoliveira added a comment - 10/Sep/09 7:40 AM

Patch looks great. The only things I would mention are minor:

1) Just remove dead code instead of commenting out
2) I still think buildURL() is a bit of a deceptive method name on ResponseUtils, since it gives no mention of the mangling that is occurring. Just looking at the name of the method without any javadoc context i would just think the method should give me a url only from the components passed in.

Both are not strong points by any means, so as you please. Nice work.

Show
Justin Deoliveira added a comment - 10/Sep/09 7:40 AM Patch looks great. The only things I would mention are minor: 1) Just remove dead code instead of commenting out 2) I still think buildURL() is a bit of a deceptive method name on ResponseUtils, since it gives no mention of the mangling that is occurring. Just looking at the name of the method without any javadoc context i would just think the method should give me a url only from the components passed in. Both are not strong points by any means, so as you please. Nice work.
Hide
Permalink
Andrea Aime added a comment - 10/Sep/09 7:45 AM

The method actually does both, it mangles and builds the URL at the same time. If there are no registered (or active manglers) it's just pure building, otherwise it's both.
Calling it just mangleURL would be strange as well, as what you pass in is not really a URL, but the pieces of it.
So... not sure... buildAndMangleURL does not sound very good.

As for the dead code, yup, I'll remove it (I thought I removed it all but some escaped my attention)

Show
Andrea Aime added a comment - 10/Sep/09 7:45 AM The method actually does both, it mangles and builds the URL at the same time. If there are no registered (or active manglers) it's just pure building, otherwise it's both. Calling it just mangleURL would be strange as well, as what you pass in is not really a URL, but the pieces of it. So... not sure... buildAndMangleURL does not sound very good. As for the dead code, yup, I'll remove it (I thought I removed it all but some escaped my attention)
Hide
Permalink
Justin Deoliveira added a comment - 11/Sep/09 8:48 AM

I actually kind of like buildAndMangleURL() as a name but i fine with just leaving it buildURL(), very minor. So I would say commit at your leisure.

Show
Justin Deoliveira added a comment - 11/Sep/09 8:48 AM I actually kind of like buildAndMangleURL() as a name but i fine with just leaving it buildURL(), very minor. So I would say commit at your leisure.
Hide
Permalink
Andrea Aime added a comment - 11/Sep/09 8:58 AM

Fixed on trunk, opened another jira to have this backported on 1.7.x

Show
Andrea Aime added a comment - 11/Sep/09 8:58 AM Fixed on trunk, opened another jira to have this backported on 1.7.x

People

  • Assignee:
    Andrea Aime
    Reporter:
    Andrea Aime
Vote (0)
Watch (2)

Dates

  • Created:
    01/Sep/09 11:26 AM
    Updated:
    11/Sep/09 8:58 AM
    Resolved:
    11/Sep/09 8:58 AM
  • Atlassian JIRA (v5.0.4#731-sha1:3aa7374)
  • Report a problem
  • Powered by a free Atlassian JIRA open source license for Codehaus. Try JIRA - bug tracking software for your team.