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

$http no longer accepts "string-like" URLs #14557

Closed

Description

@schmod

Since Angular 1.4.9, the $http service only accepts pure strings as URLs. If I pass in an object that has a toString() method, $http raises an exception:

var url = { 
   toString: function() { 
        return 'http://www.example.com';
   }
};

$http.get(url); // Throws "Error: [$http:badreq] Http request configuration url must be a string."

This is a departure from Angular's pre-1.4.9 behavior, jQuery, and XMLHttpRequest; all of which accept (and correctly interpret) the url object described above. This behavior was introduced in 6628b4f to fix #12925, and merged in #13444.

I would recommend relaxing this restriction slightly, and allow objects with a toString() method to be used as URLs.


The relevant portion of $http's source currently looks like:

if (!isString(requestConfig.url)) {
      throw minErr('$http')('badreq', 'Http request configuration url must be a string.  Received: {0}', requestConfig.url);
}

I would recommend changing it to something like this:

var isStringlike = function(val) {
    return isString(val) || (isObject(val) && isFunction(val.toString));
};
if (!isStringlike(requestConfig.url)) {
      throw minErr('$http')('badreq', 'Http request configuration url must be a string.  Received: {0}', requestConfig.url);
}

While this would result in a less-satisfactory fix for #12925 (we might throw a less descriptive error in some cases), the behavior would also be more correct.

If this looks good, I can write up a PR.

Activity

  1. schmod commented on May 4, 2016

    @schmod
    ContributorAuthor

    Some downsides to the above approach:

    • Even currently, we do not reject empty strings
    • Some objects (ie. []) serialize to an empty string
    • Object.prototype.toString does not serialize objects in a way that could ever be useful.
  2. Narretz commented on May 6, 2016

    @Narretz
    Contributor

    It's documented that http only accepts strings in the url. So I don't think we should now start to support objects with toString functions.

  3. added this to the Ice Box milestone on May 6, 2016
  4. lgalfaso commented on May 6, 2016

    @lgalfaso
    Contributor

    Agree with @Narretz the change that you point to only enforces something as it is specified in the documentation.

  5. schmod commented on May 6, 2016

    @schmod
    ContributorAuthor

    I disagree. Given that JavaScript is a dynamically-typed language, when an API document states that a method accepts a string, a developer can typically expect that the method can safely accept a string, or any other value that can be safely coerced to a string, unless the documentation specifically notes that the library performs an explicit type-check.

    To pick one very obvious example, most of Angular's API methods that accept boolean values will happily accept any truthy or falsy values. Very few other string parameters are type-checked, and Angular generally performs very little type-checking at all; only performing such checks when the coercion would be invalid, nonsensical, or unsafe.

    By introducing this change, Angular broke a contract about how developers expect a dynamically-typed language to work. While the change is technically consistent with the existing documentation, it is not consistent with how similarly-documented functions in Angular behave.

    Finally, I would argue that the rationale for introducing this change (displaying a helpful error message when a developer passes in an obviously-invalid URL) does not appear to have been actually satisfied by the changes that were introduced, nor is the stated rationale sufficient for breaking existing usages of the library (without explicitly noting it as a breaking change).

  6. schmod commented on May 6, 2016

    @schmod
    ContributorAuthor

    Workaround for anybody stumbling across this page:

    $provide.decorator('$http', function($delegate){
        var newHttp = function(cfg){
            if(cfg.url instanceof Object){
                cfg.url = cfg.url.toString();
            }
            return $delegate(cfg);
        };
    
        angular.extend(newHttp, $delegate);
    
        angular.forEach(['get', 'delete', 'head', 'jsonp'], function(method){
            newHttp[method] = function(url, cfg){
                return newHttp(angular.extend({}, cfg || {}, {
                    method: method,
                    url: url
                }));
            };
        });
    
        return newHttp;
    });
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions