Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

$resource POST removing trailing slash #992

Closed
msurdi opened this issue May 27, 2012 · 52 comments
Closed

$resource POST removing trailing slash #992

msurdi opened this issue May 27, 2012 · 52 comments

Comments

@msurdi
Copy link

msurdi commented May 27, 2012

I have a module where I define a service like this:

.factory("Charts", function ($resource) {
return $resource("/api/charts/:chartId", {
}, {});
})

Then, on the controller I do:

var chart = new Charts({
name:$scope.name,
query:$scope.query,
since:$scope.since,
since_unit:$scope.since_unit,
interval:$scope.interval
}
);
chart.$save();

And on the server I'm receiving the POST to /api/charts instead of /api/charts/ that is what (I think) it should be.

There is a discussion in the list:

https://groups.google.com/forum/?fromgroups#!topic/angular/taypgj_D3YQ

@umurkontaci
Copy link
Contributor

Pull request on this issue: https://github.com/angular/angular.js/pull/1064/files

@dandoyon
Copy link
Contributor

we just ran into this with django, it was a bit of a tussel, with angular stripping off the slash and django (by default) was adding slash it back end and doing a redirect. Even after changing django default, I ran into an inconsistency with django-tastypie and ended up modifying ngResource to append a slash to make all parties happy.

a little differen than fastreload's

return url + "/" + (query.length ? '?' + query.join('&') : '');

I think maybe an param option might be nice.

--thx

dan

@umurkontaci
Copy link
Contributor

I have done a quick hack on the issue but what I think is that frameworks should treat urls as is. Adding or removing a character from url actually means creating a completely new url and hoping that server will understand what we want.

@dandoyon
Copy link
Contributor

I think the convenience of providing the hash with bind elements (as opposed to having a url template), means, that angular needs to muck with the url. I think they tried to cover too many cases and instead should probably just tell the user. What the url will look like in the various cases or provide options to dictate what should happen. IMHO

@umurkontaci
Copy link
Contributor

Yes, but they could also make their internal interpretation of the urls and use the original url while contacting server. I have not examined the inner workings of url interpretation and routing yet this still does not legitimize changing urls without explicit order from user/coder.

Each server can have different types of interpretation urls and a client side framework should be able to communicate with them with no hassle or source changing. For example, Django has APPEND_SLASH setting which uses HTTP 302 redirection to append slash to urls without slash. This works with GET method but not with others (POST,PUT,DELETE) because redirection cannot and will not pass the data to the new URL.

@dandoyon
Copy link
Contributor

I think these are great points to bring up. Don't know if you saw but there was a fairly long thread (a number of months ago) on ngResource rewrite and request from comments from Misko. Given the diverse response, I he decided to table things until future time.

@umurkontaci
Copy link
Contributor

I haven't seen that thread, I am also not using angularjs for that long time, yet the claims that I have said are not design philosophies which is not related to a particular framework (as you said). Hopefully developers will see this issue and change their designs until new development cycle.

@stryderjzw
Copy link

Is there any more discussion on this? It would seem strange that the framework would enforce a certain type of url.

@umurkontaci
Copy link
Contributor

Not that I know. No active developers have responded to this issue yet. I think that they would agree to that it is wrong to enforce a certain type of url but this part of the framework (ngResource, http) might not be loosely coupled with the rest of the framework and it may require more than a couple of changes for it to work. But that's just a hopeful assumption.

@mhevery
Copy link
Contributor

mhevery commented Aug 21, 2012

There are many deficiencies in $resource. We are trying to figure out what
to do with it. in the meantime we recommend that you create resources
manually, which gives you full control over everything.

See:
http://stackoverflow.com/questions/11850025/recommended-way-of-getting-data-from-the-server/11850027#comment15909222_11850027

On Tue, Aug 21, 2012 at 4:20 AM, Umur Kontacı notifications@github.comwrote:

Not that I know. No active developers have responded to this issue yet. I
think that they would agree to that it is wrong to enforce a certain type
of url but this part of the framework (ngResource, http) might not be
loosely coupled with the rest of the framework and it may require more than
a couple of changes for it to work. But that's just a hopeful assumption.


Reply to this email directly or view it on GitHubhttps://github.com//issues/992#issuecomment-7898116.

@umurkontaci
Copy link
Contributor

That would be a proper workaround meanwhile. Thanks for the heads up

@stryderjzw
Copy link

Thanks for the responses! Sorry to ask this here, but following the example, Book is undefined for me in my controller. Any suggestions?

@mhevery
Copy link
Contributor

mhevery commented Sep 4, 2012

@stryderjzw the factory is missing return Book, I updated the SE.

@btford
Copy link
Contributor

btford commented Aug 24, 2013

As part of our effort to clean out old issues, this issue is being automatically closed since it has been inactivite for over two months.

Please try the newest versions of Angular (1.0.8 and 1.2.0-rc.1), and if the issue persists, comment below so we can discuss it.

Thanks!

@ivancamilov
Copy link

Hi. This issue still persists in 1.0.8. I'm using $resource to get content from a third-party that needs the URLs to have a trailing slash, and all the requests are being made without the slash.

Can we reopen this issue?

@btford btford reopened this Aug 24, 2013
@mgol
Copy link
Member

mgol commented Sep 23, 2013

I'll just add this is also a problem in 1.2.0 rcs. This causes problems with many backends that require a trailing slash at the end of requests, $resource shouldn't be stripping that (or at least it should be possible to configure it so that it puts the trailing slash instead of removing it).

@mik01aj
Copy link

mik01aj commented Sep 23, 2013

This is a big problem when using a django backend, which uses URLs with slashes by default. In most cases it responds with a redirect to an URL with trailing slash (which is OK, aside from slowing down the app a bit), but it's impossible when we're sending POST data (and that's a real problem).

@aleksihakli
Copy link

Is there a reason for $resource not respecting the trailing slash? As reported above this is a real issue with Django's POST handling as the server just responds 301 Moved Permanently for a request missing the slash.

@mgol
Copy link
Member

mgol commented Sep 27, 2013

We resorted to use a forked version of ngResource for now, with this line:

url = url.replace(/\/+$/, '');
commented out.

@seubert
Copy link

seubert commented Sep 29, 2013

@btford Can we get an explanation from a core dev regarding this? It's a lingering and festering issue that boils down to one line. I can't find any real reason for it and it's quite obviously a problem for multiple people. It shouldn't really be necessary for me to go use Restangular or drop down to $http just because of this one thing, IMO.

Thanks!

@ivancamilov
Copy link

@mzgol you should send a pull request. As for @seubert's idea, I'd like to know as well why striping the slash is a good idea. Looks like django users are having a hard time with this.

@seubert
Copy link

seubert commented Sep 29, 2013

To be clear, while Django users may be the most visible developers encountering this, unless there's a good reason, $resource should not be changing URLs. A slash is often important to a server.

@mgol
Copy link
Member

mgol commented Sep 29, 2013

@ivancamilov It's not really worth creating a pull request; our change is dead simple, any core developer could apply it in a second. This might be more of a political issue than time-related.

@aleksihakli
Copy link

It would indeed be nice to get some clarifying to why striping occurs, is it because of the parameter passing to the URL builder? At least supply user an option to turn it off in the module.

@mpaolini
Copy link

with angular 1.2.0 adding a space at the very end of the url fixes it: {url: '/user/:id/ '} [edited] this only works for calls with no querystring params

@telminov
Copy link

@mpaolini Maybe I'm wrong, but adding space at the end breaks query with parameters (angularjs 1.2.0 rc3):

with space:

qqApp.factory('Dialing', ['$resource', ($resource) ->
    return $resource('/qq/dialing/:dialingId/ ')                       #space
])
Dialing.query({org: org.id})    # ---> GET  /qq/dialing/%20?org=1038   - incorrect
dialing.$save()                 # ---> POST /qq/dialing/               - correct

without space:

qqApp.factory('Dialing', ['$resource', ($resource) ->
    return $resource('/qq/dialing/:dialingId/')                       # without space
])
Dialing.query({org: org.id})    # ---> GET /qq/dialing/?org=1038     - correct
dialing.$save()                 # ---> POST /qq/dialing              - problem for me

@mpaolini
Copy link

@telminov you are right, in the end I had to remove all trailing slashes in my django urlconf and also set APPEND_SLASH = False in my django settings to make angular happy

@Cinamonas
Copy link

RC3 still has this issue and it would be awesome if ngResource respected the exact URL it is provided.

@mgol
Copy link
Member

mgol commented Oct 28, 2013

@Cinamonas well, they obviously won't change that for 1.2.0 as it's a breaking change and a lot of code could break because of that. Too late for that now.

@mik01aj
Copy link

mik01aj commented Oct 28, 2013

@mzgol Adding an option for opt-out wouldn't break anything.

@mgol
Copy link
Member

mgol commented Oct 28, 2013

@mik01aj Ah, that's true. But it won't make it to 1.2.0 anyway, they're not accepting new features now.

@tamakisquare
Copy link
Contributor

@mzgol - I went ahead with the approach you mentioned in your earlier comment (ie. commenting out the line where the trailing slash gets stripped). You also mentioned that a lot of code could break because of that. I couldn't think of any cases that this change would break anything. Could you please enlighten me with a few examples? Thanks.

@mgol
Copy link
Member

mgol commented Oct 28, 2013

@tamakisquare That's simple, any code that would pass URLs with trailing slashes in some places and without in the others would break if the normalization process would be turned on. '$resource' will either have to be changed in a non-patch version bump or via adding an option to disable normalization but not by default.

@blaise-io
Copy link
Contributor

Please add the option to disable normalization.

@casio
Copy link

casio commented Nov 12, 2013

+1 for adding an option. This seems too dumb an issue to keep.

FWIW you can hack the behavior via double escaping the trailing slash, like:
$resource('/your/sane/api/endpoint\\/')

@teddyhwang
Copy link

@casio thanks for the tip - I just ran into this issue with a Flask backend.

@nvie
Copy link
Contributor

nvie commented Nov 27, 2013

@casio Thanks for that tip. This workaround seems to break in Firefox, though.

@mr-love
Copy link

mr-love commented Dec 4, 2013

As @nvie has indicated the workaround by @casio does not work in firefox,
I have come up with another solution that works in ff.
Add a variable onto the end of the url:
$resource('/your/sane/api/:endpoint')
and then set the variable with the trailing slash:
params : {endpoint : "endpoint/",}
but then again this isn't very nice either.

@tamakisquare
Copy link
Contributor

I took a different approach to remedy this incompatibility issue with my django backend. Just thought to share with the folks encountering the same problem for the time being. I wrote a custom response interceptor that adds the trailing slash if the url doesn't already have one. Note that the interceptor skips the requests that are configured to use $http cache.

@mpaolini
Copy link

mpaolini commented Dec 4, 2013

I think the best fix is adding APPEND_SLASH = False in your django settings.

@nvie
Copy link
Contributor

nvie commented Dec 4, 2013

That sometimes isn't an option. The best fix would be having this be configurable in the $resource() constructor, e.g.:

$resource(url[, paramDefaults][, actions][, options]);

Where options could be:

{
  trailingSlash: true/false/undefined,
}

When the trailingSlash option isn't specified, or is undefined, it's the current behaviour. When true, it could always append if necessary, when false, always stripped when necessary.

@jdhiro
Copy link
Contributor

jdhiro commented Dec 6, 2013

This problem isn't just for Django people. We have a custom PHP webservice that was created with all trailing slashes.

@nvie
Copy link
Contributor

nvie commented Dec 29, 2013

My pull request to attempt to fix this for once and for all is #5560.

This should support the following configuration:

app.config(function($resourceProvider) {
  $resourceProvider.defaults.stripTrailingSlashes = false;
});

or per instance, using a new, fourth, optional argument to the $resource() constructor:

var CreditCard = $resource('/some/:url/', ..., ..., {
    stripTrailingSlashes: false
});

Any feedback, anyone?

@j0hnsmith
Copy link

This is the stupidest bug ever, angular has no business editing urls that have been explicitly defined by the developer. And why was this ever necessary in the first place? And 2 years to fix this?

@caitp
Copy link
Contributor

caitp commented Jan 20, 2014

Thanks for being super constructive @j0hnsmith :) This has been triaged for 1.2.x, and it is potentially a breaking change for people who are depending on this functionality unwittingly!

If you can put together a PR to make this work for you, you could let people dog-food it so that we can get a bit more certainty of it, and that would be awesome! We can try to look at it for 1.2.10, if not 1.3

(Oh, and just to correct you, this isn't really "angularjs" core, this is ngResource, which you are not at all forced to use with Angular)

@j0hnsmith
Copy link

@caitp Sorry, don't mean to sound negative, I was just totally astonished by the situation, should've waited 5 mins before I pressed the comment button. After I posted the comment I did see that #5560 has been triaged. I look forward to the change making the grade.

@brian-montgomery
Copy link

Until @nvie 's patch is in, the following will do for a workaround, even in firefox.

  • Note: This will remove an ending backslash from all urls. If you need that functionality in your application, don't use this.
  • Django users: this will be making it into djangular very soon, when settings.APPEND_SLASH is true.
angular.module('ngResource').config([
    '$provide', '$httpProvider',
    function($provide, $httpProvider) {
        $provide.decorator('$resource', function($delegate) {
            return function() {
                if (arguments.length > 0) {  // URL
                    arguments[0] = arguments[0].replace(/\/$/, '\\/');
                }

                if (arguments.length > 2) {  // Actions
                    angular.forEach(arguments[2], function(action) {
                        if (action && action.url) {
                            action.url = action.url.replace(/\/$/, '\\/');
                        }
                    });
                }

                return $delegate.apply($delegate, arguments);
            };
        });

        $provide.factory('resourceEnforceSlashInterceptor', function() {
            return {
                request: function(config) {
                    config.url = config.url.replace(/[\/\\]+$/, '/');
                    return config;
                }
            };
        });

        $httpProvider.interceptors.push('resourceEnforceSlashInterceptor');
    }
]);

@jkosir
Copy link

jkosir commented Mar 6, 2014

I've been dealing with this issue while developing server-side support for $resource in Django. (crud view in django-angular)

The simplest workaround for Django is to not use trailing slashes in views used with $resource.
Note that APPEND_SLASH only appends trailing slash when the non-trailing version of the url doens't match any regex in the urlconf.
But since not using trailing slashes is kinda anti-convention, it's better to make them optional rather than removing them.

E.g.

url(r'^angular/?$', AngularView.as_view()),

I'm in favour of that rather than monkey-patching $resource js. Anyway I hope this will be fixed soon.

@FrancisVarga
Copy link

is this still open?!

@mgol
Copy link
Member

mgol commented Apr 9, 2014

Fixed via #5560.

@mgol mgol closed this as completed Apr 9, 2014
@btford
Copy link
Contributor

btford commented Apr 9, 2014

🎊

@jfbrennan
Copy link

Unreal. Spent I don't want to admit how long wasting my time trying to figure out what I was doing wrong only to find this. Why on earth was an explicitly set string being messed with in the first place?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests