Regular expression problem on Windows environment

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Browse to a directory

What is the expected output? What do you see instead?
The path to the directory must be relative, and the view should display the 
directory listing.

What revision of the FileBrowser are you using?
3.3 branche, last commit

What version/revision of Django are you using?
1.3 with Python 2.7

Please provide any additional information below.
In the file base.py, at the line 246 (def _directory(self):), the 
os.path.join(MEDIA_ROOT, DIRECTORY) should be escaped with re.escape :
re.escape(os.path.join(MEDIA_ROOT, DIRECTORY))

Original issue reported on code.google.com by Fossati....@gmail.com on 22 Mar 2011 at 8:00

[GoogleCodeExporter]

Same issue at the line 252.

Original comment by Fossati....@gmail.com on 22 Mar 2011 at 8:07

[GoogleCodeExporter]

I´ve absolutely no clue about windows and how it handles paths, so I need some 
assistance with this issue.

– are you sure about that solution? shouldn´t we use os.path.normpath?
– besides, if we change this the way you´re suggesting we need to change a 
couple of other joins as well, right?
– is everything else working as expected?

Original comment by sehmaschine on 22 Mar 2011 at 8:21

[GoogleCodeExporter]

It's not realy about Windows path handling, it's about using a regexp with 
"unsecure" string. When the string contains special character (like \ in 
windows path), it is not escaped and the re.compile doesn't work.

For the os.path.normpath, it should be used everytime we are about to open a 
file.

For other problems, the detail view doesn't show the URL of the file as 
expected. It should be the same problem. I will continue to check if there are 
another issues in Windows.

Original comment by Fossati....@gmail.com on 22 Mar 2011 at 9:17

[GoogleCodeExporter]

For me it works well to use re.escape everywhere MEDIA_ROOT and DIRECTORY are 
used (Django 1.3, Python 2.6, FileBrowser 3.3 r681)

base.py: lines 184, 189, 247, 252
functions.py lines 50, 64

I'm going to write some tests in the next days

Original comment by andreas....@moccu.com on 31 Mar 2011 at 4:57

[GoogleCodeExporter]

While testing, os.path.relpath came to my mind. I think it is a better solution 
because it can handle mixed '/' and '\' in Windows paths.

I'm not sure how to handle line 50 in functions.py. The docstring says the 
value has to be relative to MEDIA_ROOT so the regex shouldn't be necessary.

Note: My tests are kind of hackish but I wanted to change as little as possible 
in base.py. Maybe someone can come up with better tests =). Don't forget to 
import the test class in tests/__init__.py if you want use them.

Original comment by andreas....@moccu.com on 5 Apr 2011 at 3:14

Attachments:

• fileobject_path_operations.diff
• fileobject_path_operations_test.diff

[GoogleCodeExporter]

Original comment by sehmaschine on 30 May 2011 at 10:35

• Changed state: Started

[GoogleCodeExporter]

Issue 332 has been merged into this issue.

Original comment by sehmaschine on 30 May 2011 at 10:36

[GoogleCodeExporter]

sorry, but the patch doesn´t work for me. and I´m not sure os.path.relpath is 
the right solution ... I´m getting paths with "../" all over the place.

Original comment by sehmaschine on 30 May 2011 at 12:35

• Changed state: Discussion

[GoogleCodeExporter]

forget my last comment ... test are fine, thanks.

Original comment by sehmaschine on 30 May 2011 at 5:17

[GoogleCodeExporter]

I´m having no luck with this ticket ... os.path.relpath doesn´t seem a good 
solution to me, because the paths are relative (taking account of the current 
path). any help is appreciated.

thanks,
patrick

Original comment by sehmaschine on 1 Jun 2011 at 4:36

[GoogleCodeExporter]

I´m not familiar with windows-paths, but shouldn´t
filebrowser.base.DIRECTORY = 'filebrowser/uploads/'
be
filebrowser.base.DIRECTORY = 'filebrowser\\uploads\\'

Original comment by sehmaschine on 2 Jun 2011 at 11:13

[GoogleCodeExporter]

Issue 346 has been merged into this issue.

Original comment by sehmaschine on 2 Jun 2011 at 12:32

[GoogleCodeExporter]

Issue 345 has been merged into this issue.

Original comment by sehmaschine on 2 Jun 2011 at 12:33

[GoogleCodeExporter]

Issue 343 has been merged into this issue.

Original comment by sehmaschine on 2 Jun 2011 at 12:33

[GoogleCodeExporter]

Python on Windows accepts '/' as well as '\\' while '\\' looks more like 
Windows. However os.path.normpath() converts '/' to '\\'.

Original comment by andreas....@moccu.com on 2 Jun 2011 at 12:34

[GoogleCodeExporter]

thanks for the clarification.

if someone´s able to fix this ticket, I´m more than happy.
with the tests failing, I´m not going to implement the current version. 

Original comment by sehmaschine on 2 Jun 2011 at 1:07

[GoogleCodeExporter]

Can you tell me which tests are failing? Django version/OS?

Original comment by andreas....@moccu.com on 2 Jun 2011 at 1:39

[GoogleCodeExporter]

windows-tests (you need to un-comment these tests). django-version 1.3, OSX 
(but the OS shouldn´t matter since the tests use ntpath).

we´re having different patches here ... e.g. re.escape (see #346,#343,#345), 
others use os.path.relpath. I´m not sure if either of the approaches is fine 
(although using relpath seems the wrong way to go).

Original comment by sehmaschine on 2 Jun 2011 at 1:52

[GoogleCodeExporter]

Issue 349 has been merged into this issue.

Original comment by sehmaschine on 3 Jun 2011 at 2:17

[GoogleCodeExporter]

Issue 347 has been merged into this issue.

Original comment by sehmaschine on 4 Jun 2011 at 6:09

[GoogleCodeExporter]

Issue 348 has been merged into this issue.

Original comment by sehmaschine on 4 Jun 2011 at 6:09

[GoogleCodeExporter]

to everyone working on this issue and sending patches, please let me know:
a) if tests work/break
b) if we need to extend the tests and why we need to do so
c) why your solution is good (and what the drawbacks/alternatives are) ... 
compare it with already given patches
and finally, please explain your patches.

problem is:
i´ve implemented windows-patches several times only to find out that they´re 
actually not working. and I don´t have the time to add/remove these patches 
once or twice a week (and add broken tags/releases).

thanks for your understanding.

Original comment by sehmaschine on 4 Jun 2011 at 6:18

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Why you just don't do self.head.replace(os.path.join(MEDIA_ROOT, DIRECTORY), 
'')?

Original comment by iElect...@gmail.com on 4 Jul 2011 at 12:15