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

bug in os.path.join #28

Open
QuellaLiu opened this issue Jul 10, 2017 · 10 comments
Open

bug in os.path.join #28

QuellaLiu opened this issue Jul 10, 2017 · 10 comments

Comments

@QuellaLiu
Copy link

QuellaLiu commented Jul 10, 2017

In ‘utils.py’,

Line 22    curdir = os.path.join(basedir, dir)
Line 42    outfile = open(os.path.join(dest, name), 'wb')

When I unzip a file which has a path in it,

unzip

The case fails. “OSError: [Errno 13] Permission denied: '/ram'”
That’s because the method ‘os.path.join’ does not work as we expect.
For example:

>>> basedir = '/home/ute/ta_kiss_files/example/BTS9_1011_CCNTrace'
>>> dir = '/ram'
>>> curdir = os.path.join(basedir, dir)
>>> print curdir
/ram

After I change the utils.py as below,

Line 22    curdir = os.path.join(basedir, dir)
Change to  curdir = basedir + '/' + dir
Line 42    outfile = open(os.path.join(dest, name), 'wb')
Change to  outfile = open(dest + '/' + name, 'wb')

Case passes, because,

>>> curdir = basedir + '/' + dir
>>> print curdir
/home/ute/ta_kiss_files/example/BTS9_1011_CCNTrace///ram
>>>

That is what we expect.

@vkosuri
Copy link
Collaborator

vkosuri commented Jul 10, 2017

@QuellaLiu Thanks, could you please submit PR with your suggested changes?

@tminakov
Copy link

os.path.join() does work as expected, it is supposed to break the concatenation at the first absolute path. From the docs:

If a component is an absolute path, all previous components are thrown away and joining continues from the absolute path component.

Doing it manually - with basedir + '/' + dir is very dangerous, may easily produce invalid paths (that's why that module and function exist on the first place :)

I suspect it's a combination of the construct in the particulars zip, plus this code in utils.py:56

elif '/' in name:
    path = name[0:name.rindex('/')]
    dirs.add(path)

Can you please attach the zip file you're seeing the bug for, plus the exact arguments you're calling the keywords/methods with? Thank you.

@LiuQiE
Copy link
Contributor

LiuQiE commented Jul 17, 2017

I submitted a PR several days ago, but can't add any reviewer. I don't know what's the next step. Who will merge the change?

@LiuQiE
Copy link
Contributor

LiuQiE commented Jul 17, 2017

my PR: LiuQiE#1

@bulkan
Copy link
Collaborator

bulkan commented Jul 17, 2017

@LiuQiE you need to submit your PR to this repo.

@LiuQiE
Copy link
Contributor

LiuQiE commented Jul 17, 2017

Sorry! I submitted a PR to this repo just now.

@LiuQiE LiuQiE mentioned this issue Jul 17, 2017
@LiuQiE
Copy link
Contributor

LiuQiE commented Jul 19, 2017

I can see the PR is merged. When will you put the latest version in https://pypi.python.org/pypi/robotframework-archivelibrary ? Thank you very much!

@LiuQiE
Copy link
Contributor

LiuQiE commented Jul 25, 2017

Hi, Bulkan, could you please update https://pypi.python.org/pypi/robotframework-archivelibrary as soon as possible? Thank you!

@bulkan
Copy link
Collaborator

bulkan commented Jul 25, 2017

@tminakov can you please comment on this PR #30 the change looks good and not manually creating paths, just want a third opinion on it.

@LiuQiE
Copy link
Contributor

LiuQiE commented Aug 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants