Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

$parse removes object keys that begin with $ (dollar sign) #1463

Closed
daryllstrauss opened this issue Oct 16, 2012 · 58 comments

Comments

@daryllstrauss
Copy link

commented Oct 16, 2012

If you attempt to send {'_id':{'$oid':'123'}} as an argument to a resource, the {'$oid':'123'} is removed from the http request that is sent.

This is a problem, because mongodb uses keys that start with $ as a way to serialize certain objects.

As far as I can tell there's no restriction on $ from the strings in a JSON object, so it seems the behavior of parse is wrong.

@jtymes

This comment has been minimized.

Copy link
Contributor

commented Nov 26, 2012

Angular is stripping the $ because it is used for Angular (mostly) properties/methods and when they are converted to JSON, they're stripped because it's extraneous.

Here's an idea for a solution:
For certain components, e.g. $resource or $http, allow a special leading sequence (such as "+$") to signal that the $ not be stripped. I don't believe this would be a breaking change.

So, for this example, '+$oid' would be stripped down to '$oid'.

Thoughts?

@daryllstrauss

This comment has been minimized.

Copy link
Author

commented Nov 28, 2012

Quoting might be a good solution.

My question is where the quoting/dequoting would occur.

I believe $http is the low level transport. From my point of view, the
ideal solution would be to have $http quote and dequote the identifiers.
That way it would be transparent to my python (mongodb) application. My
angular application would see +$oid as an identifier, but that wouldn't
be so bad.

$ would be a more traditional way to quote it.

@mstein

This comment has been minimized.

Copy link

commented Dec 3, 2012

Same issue here, writing a plugin for grails and mongodb, I was forced to parse/marshall json object manually to keep the '$xxx' keys in the object received/sent.

As a workaround for now, you can send your JSON as a string using a standard or custom json stringifier, same for the parser as the keys would be stripped from the response as well, ex:

var args = {"$oid":"xxx"};
// MongoJSON is a custom parser/stringifier, also handling a bit of mongodb '10Gen' syntax
$http.post('mongo/find', MongoJSON.stringify(args), {transformResponse:parseMongoJson}).success(function(data) {
    successCallback(data);
}).error(function(data){ errorCallback(data); });

function parseMongoJson(data, headerGetter) {
    return MongoJSON.parse(data, mongoJsonReviver);
}

Still waiting for a proper fix for this issue, actually even passing an object with "$xx" keys between scopes seems to strip those keys...

@IgorMinar

This comment has been minimized.

Copy link
Member

commented Jan 30, 2013

how about writing a custom serializer and replace the default one with $httpProvider.defaults.transformRequest ?

current implementation:

    transformRequest: [function(d) {
      return isObject(d) && !isFile(d) ? toJson(d) : d;
    }],

where toJson contains the problematic stripping json serializer:

function toJson(obj, pretty) {
  return JSON.stringify(obj, toJsonReplacer, pretty ? '  ' : null);
}
@IgorMinar

This comment has been minimized.

Copy link
Member

commented Jan 30, 2013

I'm not keen on not dropping the $* properties during serialization, but I wonder if allowing developers to whitelist properties that should not be stripped would be a sufficient solution to this issue.

@jtymes

This comment has been minimized.

Copy link
Contributor

commented Jan 30, 2013

@IgorMinar 👍 I think it is a small number of users that this impacts and allows for flexibility unlike service APIs such as the one mentioned above that tend to be near impossible to change.

@tompro

This comment has been minimized.

Copy link

commented Feb 25, 2013

👍 I think mongodb and angular will be quite a common stack. With static typed languages on the serverside (where json serialization is handled by 3rd party libs) this is especially problematic, as one has to rebuild all serialization.

@MarkTroutfetter

This comment has been minimized.

Copy link

commented Apr 23, 2013

+1 Would really like a solution to this!! Thanks!

@jspeaks

This comment has been minimized.

Copy link

commented Apr 24, 2013

This is an issue when working with MongoDB based services that are dependent on $ prefixed object properties. The framework needs to provide an escape mechanism for this.

@lukemadera

This comment has been minimized.

Copy link

commented May 17, 2013

I had the same problem (also using MongoDB on the backend) but it seems just manually stringifying the data/params prior to calling the $http service works, i.e.

opts.data =JSON.stringify(opts.data);
opts.params =JSON.stringify(opts.params);
$http(opts).success(...).error(...);
@dcolens

This comment has been minimized.

Copy link

commented Jun 17, 2013

Could this be documented at: http://docs.angularjs.org/api/angular.toJson, right now there's no mention that angular.toJson strips properties starting with a '$'.

@jamescookie

This comment has been minimized.

Copy link

commented Jun 25, 2013

I would like to see the whitelist approach taken, this would seem the simplest solution.
Then avoiding $oid removal would be trivial.
In the meantime, we had to override the behaviour like this:

angular.module('myApp', ['myDirectives']).
    run(['$document', '$http', function ($document, $http) {
        var toJsonReplacer = function(key, value) {
            var val = value;

            if (/^\$+/.test(key) && key !== '$oid') {
                val = undefined;
            } else if (value && value.document && value.location && value.alert && value.setInterval) {
                val = '$WINDOW';
            } else if (value && $document === value) {
                val = '$DOCUMENT';
            } else if (value && value.$evalAsync && value.$watch) {
                val = '$SCOPE';
            }
            return val;
        };

        angular.toJson = function(obj, pretty) {
            return JSON.stringify(obj, toJsonReplacer, pretty ? '  ' : null);
        };

        $http.defaults.transformRequest = function(d) {
            return angular.isObject(d) && !(angular.toString.apply(d) === '[object File]') ? angular.toJson(d) : d;
        };
    }]);
sappleg pushed a commit to sappleg/angular.js that referenced this issue Jun 29, 2013
Spencer Applegate
Update Angular.js toJson documentation
I included in angular.toJson's description that any properties with a leading '$' character will be stripped from the result since angular uses this notation internally for services.  There have been complaints of not knowing about this functionality until it breaks within their code.  It would be a nice addition to avoid any confusion in the future.

An example of a documentation update request is within a comment in issue angular#1463.
@krishnakumarmasuku

This comment has been minimized.

Copy link

commented Aug 21, 2013

Hey guys,
I'm new to angularjs. And i'm trying add the data to json file through angular js. If anyone of you helps me i feel very happy.
Thanks & regards,
krishh

@jspeaks

This comment has been minimized.

Copy link

commented Aug 21, 2013

krishh, github issues is not the right place or manner to request individual help on this topic. email me directly from your email account, or via twitter, and update your github profile with appropriate info if you are going to be requesting help.

@aruizca

This comment has been minimized.

Copy link

commented Aug 26, 2013

+1 for a solution to work with MongoDB without doing dodgy workarounds

@vkarpov15

This comment has been minimized.

Copy link

commented Oct 10, 2013

Gotta give this a +1 too, one of my colleagues just spent a couple hours getting blindsided by this bug.

@ninjatronic

This comment has been minimized.

Copy link

commented Oct 14, 2013

+1 for this - attempting to use angular.js with mongo (very common?) is a massive pain in the rump

@anemitoff

This comment has been minimized.

Copy link

commented Nov 7, 2013

+1 for this

@roadsunknown

This comment has been minimized.

Copy link

commented Nov 12, 2013

JSON.NET parser also uses $properties, specifically when strongly typing .NET types it uses $type. Both workarounds above work (stringify and overriding toJsonReplacer), but +1 vote for a configurable whitelist.

@shravansing

This comment has been minimized.

Copy link

commented Nov 17, 2013

Sending string instead of JSON is not the solution.
Why should the server side code be changed ?? Why should the information protocol change from JSON to string??
This is clearly an issue in Angular.js
I am using a framework with expects $ in JSON messages .. i can't fix a third party server.

BIGGEST BUG, by not allowing something in JSON

@ghost ghost assigned btford Dec 19, 2013

@standup75

This comment has been minimized.

Copy link
Contributor

commented Dec 20, 2013

+1

@stephennancekivell

This comment has been minimized.

Copy link
Contributor

commented Jan 1, 2014

+1 Im using mongodb. Im going to change my server code. Not ideal.

@Raichel

This comment has been minimized.

Copy link

commented Jan 3, 2014

+1

1 similar comment
@an-tex

This comment has been minimized.

Copy link

commented Jan 9, 2014

+1

@yarai728

This comment has been minimized.

Copy link

commented Apr 28, 2014

+1

@matthoiland

This comment has been minimized.

Copy link

commented Apr 29, 2014

+1. Not being able to use MongoDB comparison operators in a Resource is a bummer.

@joshvillbrandt

This comment has been minimized.

Copy link

commented Jun 1, 2014

+1

@a8m

This comment has been minimized.

Copy link
Contributor

commented Jun 3, 2014

@btford @caitp
Is good enough to add an optional parameter of immutability to 'toJsonReplacer' function?, and if it does(http request), object(request body) is immutable.
what do you think ?

@caitp

This comment has been minimized.

Copy link
Contributor

commented Jun 3, 2014

when did this get triaged as high priority? should try to get this checked in, imo.

@a8m --- I don't know if that's necessarily the best way to do it, but I think it would be okay if we only removed $$hashKey, and maybe some of the scope properties. However I think scope parameters are unlikely to be serialized as you generally don't send an entire scope to the backend, so it's probably not even worth worrying about.

AFAIK, $$hashKey is the only property which angular should add to objects that you might actually want to send off.

@btford

This comment has been minimized.

Copy link
Contributor

commented Jun 3, 2014

@caitp – was just wondering that.

I saw @rodyhaddad had a fix in the work for this, but it didn't implement @IgorMinar's whitelisting approach.

@caitp

This comment has been minimized.

Copy link
Contributor

commented Jun 3, 2014

@btford I was going to reply to your comment on the PR, but here's as good a place as any ---

So I don't think a whitelist is necessarily the way to go, because what this means is we're saying like, $eq is allowed because MongoDB might want to use it, but things that aren't known to be useful to mongo shouldn't be sent off.

So the reasons why that's problematic are pretty obvious, A) it's another list of things not in angular's control that needs to be maintained, and B) we're likely to exclude people using different toolchains on their server if we do that.


So, we know that Angular has a pretty limited set of keys which it adds directly to objects (not in the prototype chain) --- $$hashKey, and some scope properties like $root or $parent --- but scopes are unlikely to be JSON-ified anyways, so I don't think that's a huge problem.

My vote is to just remove own-$$-prefixed properties, or just a specific blacklist of properties which angular adds. It's the least likely to be a breaking change, and the former is the easiest to work around if it does break applications.

In other words, I think @rodyhaddad's PR should be good enough for this, the whitelist is probably not a good idea

@a8m

This comment has been minimized.

Copy link
Contributor

commented Jun 3, 2014

@caitp blacklist of properties it's good idea,
for now $http service strip keys with $ in the head..
e.g : {$foo: 'bar'} ==> {} , and $foo isn't part of the $scope

//current function
function toJsonReplacer(key, value) {
var val = value;

if (typeof key === 'string' && key.charAt(0) === '$') {
val = undefined;
} else if (isWindow(value)) {
val = '$WINDOW';
} else if (value && document === value) {
val = '$DOCUMENT';
} else if (isScope(value)) {
val = '$SCOPE';
}

return val;
}
==> should get a third parameter of immutability or check against a specific blacklist

@caitp

This comment has been minimized.

Copy link
Contributor

commented Jun 3, 2014

toJsonReplacer is called by JSON.stringify, so a third parameter to it isn't going to help much.

@caitp

This comment has been minimized.

Copy link
Contributor

commented Jun 3, 2014

And yes, I know that we're stripping anything with $ as the first character, this is not good =)

@a8m

This comment has been minimized.

Copy link
Contributor

commented Jun 3, 2014

Even easier, request.body doesn't need replacer(stringify replacer), just stringify it :-)

@btford btford removed the high priority label Jun 3, 2014

@btford

This comment has been minimized.

Copy link
Contributor

commented Jun 4, 2014

Closed via #6253.

@btford btford closed this Jun 4, 2014

bkeil added a commit to bkeil/CornerCouch that referenced this issue Jul 15, 2014
Allow $http configuration for CouchDoc.save
This commit allows additional configuration parameters to be sent for the $http request in `CouchDoc.save`.  I need to do this in order to work around angular/angular.js#1463
@poths

This comment has been minimized.

Copy link

commented Jan 28, 2015

@btford - I hope this is the right place to ask for it, but with the fix of "only stripping" $$ attributes, angular now sends $promise and $resolved attributes back to the server. What is the recommended way of stripping of these attributes now? (our JAX-RS Json to object mapping doesn't like them). Thanks for any recommendation.

@gkalpak

This comment has been minimized.

Copy link
Member

commented Jan 28, 2015

@poths: I don't know how you are using/sendng your objects, but $resource instances have an overriden toJson method that makes sure those properties are removed when calling JSON.stringify on them.
(https://github.com/angular/angular.js/blob/master/src/ngResource/resource.js#L504)

@poths

This comment has been minimized.

Copy link

commented Jan 28, 2015

@btford - thanks a lot. I didn't know where to look for that. we had an incompatible version of angular-resource, without the custom toJSON method. works great after an update.

ujibang added a commit to SoftInstigate/restheart that referenced this issue Apr 21, 2015
added workaroung for angularjs issue 1463 angular/angular.js#1463 ($o…
…id can also be passed as €oid

removed useless warning message if ref field is null
ujibang added a commit to SoftInstigate/restheart that referenced this issue Apr 28, 2015
added 2 transformers to help representing ObjectId as strings
very useful for angularjs that removes all properties whose name start with $, due to issue 1463 angular/angular.js#1463
removed old angularjs workaround
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.