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

Include path separator and absolute path when checking if destination is a child of the source. Fixes gh-287 #289

Merged
merged 2 commits into from Dec 4, 2018

Conversation

Projects
None yet
3 participants
@jmathai
Owner

jmathai commented Dec 2, 2018

When source is /path/to/file.jpg and destination is /path/t the import would fail due to simple string matching. gh-287

@coveralls

This comment has been minimized.

coveralls commented Dec 2, 2018

Coverage Status

Coverage remained the same at 93.124% when pulling 51d2d01 on dest_in_source-287 into ffb26fb on master.

@benmccann

This comment has been minimized.

Contributor

benmccann commented Dec 2, 2018

Looks good to me. Thanks for the quick fix!

@benmccann

This comment has been minimized.

Contributor

benmccann commented Dec 2, 2018

It could help to add a test demonstrating the issue in gh-287 to prevent a regression

@jmathai

This comment has been minimized.

Owner

jmathai commented Dec 4, 2018

Added test to cover this use case.

Master branch.

(elodie) ➜  elodie git:(master) ✗ nosetests tests/elodie_test.py:test_import_destination_in_source_gh_287
F
======================================================================
FAIL: elodie.tests.elodie_test.test_import_destination_in_source_gh_287
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jaisen/.virtualenvs/elodie/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/Users/jaisen/dev/elodie/elodie/tests/elodie_test.py", line 268, in test_import_destination_in_source_gh_287
    assert dest_path is not None, dest_path
AssertionError: None
-------------------- >> begin captured stdout << ---------------------
{"source": "/var/folders/ch/y7blgqys2035sbny01j9731400d9ct/T/F4H3SYQKSF/C185ZOKB13/video.mov", "destination": "/var/folders/ch/y7blgqys2035sbny01j9731400d9ct/T/F4H3SYQKSF/C185ZOKB13-destination", "error_msg": "Source cannot be in destination"}

--------------------- >> end captured stdout << ----------------------

----------------------------------------------------------------------
Ran 1 test in 0.043s

FAILED (failures=1)

dest_in_source-287 branch.

(elodie) ➜  elodie git:(dest_in_source-287) ✗ nosetests tests/elodie_test.py:test_import_destination_in_source_gh_287
.
----------------------------------------------------------------------
Ran 1 test in 4.149s

OK

@jmathai jmathai merged commit 3117a06 into master Dec 4, 2018

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@jmathai jmathai deleted the dest_in_source-287 branch Dec 4, 2018

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