Skip to content

Commit 5ef38a1

Browse files
committed
Create copy of note dict in update_note to avoid external modification
Never really paid attention to this, but because the dict is mutable we end up modifying the external reference: ``` note = {...} n, s = sn.update_note(note) note = {... plus other stuff} ``` Probably been "broken" for ages, but only made obvious when we started adding in the fields required for Simperium. --- Strictly speaking should/could create copies of the note dict in the `__add...` and `__remove...` methods, but since those are internal only I can't see it causing a problem. Add a test for it as well. Fixes: #26
1 parent 19a8c6e commit 5ef38a1

File tree

2 files changed

+25
-10
lines changed

2 files changed

+25
-10
lines changed

simplenote/simplenote.py

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ def authenticate(self, user, password):
6464
""" Method to get simplenote auth token
6565
6666
Arguments:
67-
- user (string): simplenote email address
67+
- User (string): simplenote email address
6868
- password (string): simplenote password
6969
7070
Returns:
@@ -157,27 +157,29 @@ def update_note(self, note):
157157
- status (int): 0 on success and -1 otherwise
158158
159159
"""
160+
# Let's create a copy to work with so we don't alter original
161+
note_to_update = note.copy()
160162
# determine whether to create a new note or update an existing one
161163
# Also need to add/remove key field to keep simplenote.py consistency
162-
if "key" in note:
164+
if "key" in note_to_update:
163165
# Then already have a noteid we need to remove before passing to Simperium API
164-
noteid = note.pop("key", None)
166+
noteid = note_to_update.pop("key", None)
165167
else:
166168
# Adding a new note
167169
noteid = uuid.uuid4().hex
168170

169171

170172
# TODO: Set a ccid?
171173
# ccid = uuid.uuid4().hex
172-
if "version" in note:
173-
version = note.pop("version", None)
174+
if "version" in note_to_update:
175+
version = note_to_update.pop("version", None)
174176
url = '%s/i/%s/v/%s?response=1' % (DATA_URL, noteid, version)
175177
else:
176178
url = '%s/i/%s?response=1' % (DATA_URL, noteid)
177179

178180
# TODO: Could do with being consistent here. Everywhere else is Request(DATA_URL+params)
179-
note = self.__remove_simplenote_api_fields(note)
180-
request = Request(url, data=json.dumps(note).encode('utf-8'))
181+
note_to_update = self.__remove_simplenote_api_fields(note_to_update)
182+
request = Request(url, data=json.dumps(note_to_update).encode('utf-8'))
181183
request.add_header(self.header, self.get_token())
182184
request.add_header('Content-Type', 'application/json')
183185

@@ -191,9 +193,9 @@ def update_note(self, note):
191193
return e, -1
192194
except IOError as e:
193195
return e, -1
194-
note = json.loads(response.read().decode('utf-8'))
195-
note = self.__add_simplenote_api_fields(note, noteid, int(response.info().get("X-Simperium-Version")))
196-
return note, 0
196+
note_to_update = json.loads(response.read().decode('utf-8'))
197+
note_to_update = self.__add_simplenote_api_fields(note_to_update, noteid, int(response.info().get("X-Simperium-Version")))
198+
return note_to_update, 0
197199

198200
def add_note(self, note):
199201
""" Wrapper method to add a note
@@ -380,6 +382,9 @@ def delete_note(self, note_id):
380382

381383
def __add_simplenote_api_fields(self, note, noteid, version):
382384
# Compatibility with original Simplenote API v2.1.5
385+
386+
# We are not creating a copy of the note to work with as these are only
387+
# used internally and so doesn't matter if we alter "original"
383388
note[u'key'] = noteid
384389
note[u'version'] = version
385390
try:
@@ -392,6 +397,9 @@ def __add_simplenote_api_fields(self, note, noteid, version):
392397
return note
393398

394399
def __remove_simplenote_api_fields(self, note):
400+
# We are not creating a copy of the note to work with as these are only
401+
# used internally and so doesn't matter if we alter "original"
402+
395403
# These two should have already removed by this point since they are
396404
# needed for updating, etc, but _just_ incase...
397405
note.pop("key", None)

tests/test_simplenote.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,13 @@ def test_note_get_previous_version(self):
187187
if status == 0:
188188
self.assertEqual("Hello", note["content"])
189189

190+
def test_external_note_object_does_not_get_altered(self):
191+
external_note = {'modifydate': 2.0, 'createdate': 1.0, 'tags': [], 'content': 'note'}
192+
reference_note = {'modifydate': 2.0, 'createdate': 1.0, 'tags': [], 'content': 'note'}
193+
note, status = self.simplenote_instance.add_note(external_note)
194+
if status == 0:
195+
self.assertEqual(external_note, reference_note)
196+
190197
def is_utf8(self, s):
191198
if sys.version_info < (3, 0):
192199
try:

0 commit comments

Comments
 (0)