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

Observers can mess with internal data #855

Closed
andreas-karlsson opened this Issue Mar 20, 2013 · 6 comments

Comments

Projects
None yet
3 participants
@andreas-karlsson
Contributor

andreas-karlsson commented Mar 20, 2013

This applies at least to observeChanges.changed on the server. The same "fields" object that observers receive is used to update internal data. If an observer modifies the fields argument it will mess with updating data in the observed collection.

When doing custom publication it's quite handy to just send the same fields object to the sub.changed. We were however filtering the fields object, removing properties based on access restrictions. This resulted in the observer receiving changes indefinitely from mongo polling. Deepcloning the fields object fixed this.

@cmather

This comment has been minimized.

Contributor

cmather commented Mar 21, 2013

Hi @andreas-karlsson, We agree. But just to make sure we understand what's happening would you mind linking to a reproduction? Much appreciated.

@andreas-karlsson

This comment has been minimized.

Contributor

andreas-karlsson commented Mar 21, 2013

I wrote a test to show the problem and a proposed fix. The commit below is the fix and the one before it is the test.
andreas-karlsson@ef1266a

@glasser

This comment has been minimized.

Member

glasser commented Mar 21, 2013

Thanks for the example! In fact, based on your initial report I was focusing on another part of the code (diff.js) and I didn't notice that issue. Your fix and test does look good... care to file it as a pull request?

I think there might be a further issue in Minimongo though. Specifically if you don't mutate fields itself but you do mutate a subfield of it (eg fields.foo.bar = ...) you can probably affect some internal minimongo state. (Your fix only affects the server, not the client, though it should be fully effective on the server.)

@andreas-karlsson

This comment has been minimized.

Contributor

andreas-karlsson commented Mar 21, 2013

I wrote the commits against 0.5.9 and after rebasing to latest devel I couldn't figure out how to run the tests, so I didn't want to file a PR. Still, here it is #863

@glasser

This comment has been minimized.

Member

glasser commented Mar 21, 2013

"meteor test-packages" (or "meteor test-packages mongo-livedata", etc)

On Thu, Mar 21, 2013 at 2:43 PM, andreas.karlsson
notifications@github.comwrote:

I wrote the commits against 0.5.9 and after rebasing to latest devel I
couldn't figure out how to run the tests, so I didn't want to file a PR.
Still, here it is #863 #863


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

@glasser

This comment has been minimized.

Member

glasser commented Apr 9, 2013

Merged, thanks!

@glasser glasser closed this Apr 9, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment