CAS authority should not bail when service URL is not a valid URI

[rsutphin]

If the CAS authority encounters a service URL which is not a valid URI, it throws an exception like the one below. It should handle this situation more gracefully.

A URI::InvalidURIError occurred in participants#show:

 bad URI(is not URI?): https://foo.example.edu/participants/{p_id}
 /usr/local/rvm/rubies/ruby-1.9.3-p327/lib/ruby/1.9.1/uri/common.rb:176:in `split'
 /usr/local/rvm/rubies/ruby-1.9.3-p327/lib/ruby/1.9.1/uri/common.rb:211:in `parse'
 /usr/local/rvm/rubies/ruby-1.9.3-p327/lib/ruby/1.9.1/uri/common.rb:747:in `parse'
 aker (3.0.4) lib/aker/cas/service_url.rb:36:in `service_url'
 aker (3.0.4) lib/aker/cas/service_url.rb:21:in `service_url'
 aker (3.0.4) lib/aker/cas/service_mode.rb:82:in `block in on_ui_failure'
 rack (1.3.6) lib/rack/response.rb:44:in `initialize'
 aker (3.0.4) lib/aker/cas/service_mode.rb:80:in `new'
 aker (3.0.4) lib/aker/cas/service_mode.rb:80:in `on_ui_failure'
 aker (3.0.4) lib/aker/rack/failure.rb:26:in `call'
 warden (1.2.1) lib/warden/manager.rb:130:in `call_failure_app'
 warden (1.2.1) lib/warden/manager.rb:116:in `process_unauthenticated'
 warden (1.2.1) lib/warden/manager.rb:47:in `call'
 aker (3.0.4) lib/aker/rack/setup.rb:63:in `call'

[hannahwhy]

To me, this looks more like some method isn't properly escaping path components.

[rsutphin]

This is exception is the result of navigating to the literal URL https://foo.example.edu/participants/{p_id} where foo.example.edu is an Aker-protected app using the CAS UI mode. The {/} shouldn't be in the URL (attempting to navigate there is a mistake), but Aker should also not throw an exception over it.

[hannahwhy]

I guess I'm wondering what the proposed fix for this is; https://foo.example.edu/participants/{p_id} could very well be valid for some application. (It'd have to be https://foo.example.edu/participants/%7Bp_id%7D, of course.)

It seems to me that Aker can avoid this particular exception by escaping attempted_path or ensuring that service_url always receives an escaped path. Was that part of the plan?

[rsutphin]

Yeah, I'm not sure what the best solution would be. (This is why I didn't include a proposed one in the ticket.)

I'm kind of surprised that the browser passes the non-escaped URI to the server. I've verified that both Firefox and Safari do, though.

It seems to me that Aker can avoid this particular exception by escaping attempted_path or ensuring that service_url always receives an escaped path.

I don't think this will work. What if attempted_path is already escaped?

1.9.3p327 :007 > URI.escape('/a/b/c/%7B4}')
 => "/a/b/c/%257B4%7D" 

I'm inclined to just handle the exception with a fallback, but I'm not sure what the fallback should be.

[hannahwhy]

It's possible to detect escaped URIs:

1.9.3p327 :018 > '/a/b/c/{4}'.match /#{URI::REGEXP::PATTERN::ESCAPED}/
 => nil 
1.9.3p327 :019 > '/a/b/c/%7B4%7D'.match /#{URI::REGEXP::PATTERN::ESCAPED}/
 => #<MatchData "%7B"> 

The mechanism is pretty obscure, though, and it certainly isn't documented. I get the feeling that ESCAPED is really meant for internal use.

It also won't catch partially escaped paths like your example, though I think in that case it's safe to say that the user-agent passing the URL is misbehaving, and we should just abort the request with an error like Aker::Cas::InvalidServiceURL. It's still an exception, but it's more informative.

I think, however, that both of these approaches are too much. The point of service_url, as far as I can tell, is to scrub out a service ticket from the query string, and we can do that with a regexp:

attempted_path.gsub(/ticket=ST-[^&]+&?|&ticket=ST-[^&]+/, '')

This eliminates the need to construct a URI object. (We can construct a URI-as-string via string concatenation in the no-attempted-path-given case.)