angular / angular.js

AngularJS - HTML enhanced for web apps!
https://angularjs.org
MIT License
58.82k stars 27.51k forks source link

encodeUriSegment in resource encodes params, should be optional #1388

Open fredrikbonander opened 12 years ago

fredrikbonander commented 12 years ago

When having a $resource and sending a param to be used in the url, it would be nice if there was an option not to encode it. OOB it encodes (in this case the "path") param before it's match agains the url. For example

// In SomeResourceName factory:
$resouce('/:path',  { path: 'default.json' }, ...)
// Useing SomeResourceName
SomeResourceName.get({ path: 'game/mygame.json' })

This will result in a call to url "/game%2Fmygame.json" instead of "/game/mygame.json".

There's a quick-fix-workaround:

// In angular-resource.js and method encodeUriSegment
  function encodeUriSegment(val) {
    return encodeUriQuery(val, true).
      replace(/%26/gi, '&').
      replace(/%3D/gi, '=').
      replace(/%2B/gi, '+'). 
      replace(/%2F/gi, '/'); // <--- Add this line
  }

I have no idea what will break, but as of know it works for me. One could also hijack the actions argument in ResourceFactory and pass a skip encode flag to the Route constructor's defaults argument to tell it to skip the encoding.

fredrikbonander commented 12 years ago

Created a gist with the changes for hijacking :) https://gist.github.com/3749345

manuelsantillan commented 11 years ago

+1 to this. Provides greater flexibility. In our use case, we are using spring data rest for the backend and search methods are path params that we need to map to actions, ideally in the same resource object as for the basic CRUD ops.

georgiosd commented 11 years ago

+1. Our ids contain slashes (we use RavenDB) and it would be a clean solution if it wasn't encoded

saden1 commented 11 years ago

Making it optional is fine but please insure that it's enabled by default.

It is ill advised to not encode URIs. Not doing so might seem "clean" now but I assure you it will bite you in the ass in this life or the next.

toddb commented 11 years ago

If I read http://www.ietf.org/rfc/rfc3986.txt correctly, we should be allowed the unencoded slash for the fragment.

3.5. Fragment

The fragment identifier component of a URI allows indirect identification of a secondary resource by reference to a primary resource and additional identifying information. The identified secondary resource may be some portion or subset of the primary resource, some view on representations of the primary resource, or some other resource defined or described by those representations. A fragment identifier component is indicated by the presence of a number sign ("#") character and terminated by the end of the URI.

 fragment    = *( pchar / "/" / "?" )

... The characters slash ("/") and question mark ("?") are allowed to represent data within the fragment identifier. Beware that some older, erroneous implementations may not handle this data correctly when it is used as the base URI for relative references (Section 5.1).

Use case: $location.hash('/secondary-resource')

ProLoser commented 11 years ago

I'm just curious, is it possible to avoid this issue by double-encoding your params?

toddb commented 11 years ago

Sorry, it then makes it even worse. Just to confirm I tried out a number of opens even though it felt futile!

'/', '%252F' --> %25252F

Also,

/ %2f --> %252F '/', '//' --> %2F%2F

Thanks for the thought. Request still stands.

sephie commented 11 years ago

Ran into this today. +1 I might not like it, but our id's contain slashes as well. This needs to be optional.

stevermeister commented 11 years ago

+1

retailpoint commented 11 years ago

+1

chelexey commented 11 years ago

+1

mrzepinski commented 10 years ago

+1

e0ipso commented 10 years ago

+1

jchonde commented 10 years ago

+1

jeffbcross commented 10 years ago

I see mention of a database (RavenDB) that makes the current implementation problematic, but I'd expect there to be an intermediary server that expects encoded components, and decodes them to the correct ID on the backend. Could someone provide a more concrete use case on where forced encoding is causing problems (sorry I don't have more context on RavenDB)?

We hesitate to allow this with the current design of $resource, because it'd make it to easy for user input to affect the path of a request.

toddb commented 10 years ago

I use $location for a REST/hypermedia design and not RavenDB (or $resource). I have had to work with a patched version of the library.

I am also seeking confirmation that in fact this proposal does actually follow the RFC too -see my comment above

I might also be a bit rusty around the library - so big apologies if I am wrong - there are also two implementations of one in resource at L332 and one in Angular at 1071.

I just need the one in Angular because it is what is used by $location ;-) (yes, I see the problem here so I wouldn't suggest haven't different implementations). Apologies if I have any incorrect analysis.

davecap commented 10 years ago

+1 Anybody have a workaround for constructing resource URLs containing forward slashes?

toddb commented 10 years ago

As per the patch line mentioned above in the files the two implementations also mentioned above. It's a pain.

LFDM commented 10 years ago

Is there any movement on this? Just add a point where I could really use such an option myself.

patricklehmann commented 10 years ago

hey there,

i've the same problem but the gist doesn't make it work for me. where do i have to place this encodeUirSegment method or where can i place this option? :/

connorbode commented 10 years ago

I was using an HTTP interceptor to transform URLs. It was working until I tested on IE11. Angular breaks on XHR requests to URLs including % in IE11. Not sure about IE10. Angular is supposed to support IE9+ (reference)

Guess I'm going back to the modified ngResource..

caitp commented 10 years ago

@connorbode --- dude, like it was mentioned in your issue, write a failing test case =) This path SHOULD already be covered, so there's two possibilities, either it's not covered and should be, or you're doing something to break this.

Lets find out which it is!

an-dev commented 10 years ago

+1

ksokol commented 10 years ago

+1

rkakrik commented 10 years ago

+1

Impossible to work with CouchDB and design-documents (_design/cafehub/_view/menu_items)

connorbode commented 10 years ago

Can you guys +1 my pull request instead of this issue? It fixes the issue but has been generally ignored since its submission. https://github.com/angular/angular.js/pull/7940

grhegde09 commented 10 years ago

+1 Really need this to interact with some legacy webservices which do not perform URLdecode

rossbayer commented 10 years ago

+1

There are definitely viable, useful circumstances where a user will want to disable the encoding of URL parameters (for instance, where you want to append to a base URL a path to a resource).

pachoulie commented 9 years ago

+1

geoyws commented 9 years ago

+1

jcfontecha commented 9 years ago

+1

rashad803 commented 9 years ago

+1

mprobst commented 9 years ago

@toddb just FYI, your spec quote refers to the fragment part of a URL, i.e. the #foo/bar part, where escaping a slash is indeed not required or appropriate. But this bug is about escaping a slash in the main path part of the URL. In general, I don't think the RFC applies to this, how Angular's $resource service handles pattern matching and URL construction really doesn't concern the RFC much.

Not escaping slashes in a regular :param style parameter has the problem that it makes the pattern non-invertible. E.g. if you have /x/:param, and you allow generating /x/y/z for {param: 'y/z'}, the resulting /x/y/z will fail to parse against the URL pattern.

IMHO it might make sense to have a feature in URL patterns to accept a star pattern that eats slashes, e.g. "/:param1/:param2/*pathParam", where *pathParam would eat slashes in the URL. For * params, it'd then make sense to also accept slashes without escaping them.

toddb commented 9 years ago

@mprobst - you are quite correct that I was logging a bug only around the fragment. The code as I use it also affects the fragment. I am not using the $resource service but rather $location. If my memory serves me there are a couple of implementations of the uriSegment code.

The patch by @connorbode from a quick read will address the issue. Cheers

adamhadani commented 9 years ago

+1

nmehta6 commented 9 years ago

+1

paulistoan commented 9 years ago

+1

strokyl commented 9 years ago

+1

kinoakter commented 9 years ago

+1

tonycapone commented 9 years ago

+1

jkremser commented 9 years ago

+1

pavolloffay commented 9 years ago

+1

patou commented 9 years ago

You can use the same syntaxe like in the ui-router lib : http://angular-ui.github.io/ui-router/site/#/api/ui.router.util.type:UrlMatcher

jiananshi commented 9 years ago

+1

it also happens in location

DaAwesomeP commented 8 years ago

:+1:

TheSameSon commented 8 years ago

+1

jiananshi commented 8 years ago

I'm current doing a decode in order to send my original url to backend

.factory('decodeUriSegment', () => {
    return (url) => {
      return url.replace(/@/g, '%40')
        .replace(/:/g, '%3A')
        .replace(/\$/g, '%24')
        .replace(/,/g, '%2C')
        .replace(/\+/g, '%20');
    };
  });
tekstrand commented 8 years ago

+1

kishorekumaru commented 8 years ago

+1

zapolnoch commented 8 years ago
app.config(function($resourceProvider) {
    $resourceProvider.defaults.stripTrailingSlashes = false;
});

https://github.com/angular/angular.js/pull/5560