Opened 10 years ago

Closed 8 months ago

#10521 closed defect (invalid)

notebook upload of zipped worksheets can't deal with multiple worksheets with the same name

Reported by: jason Owned by: jason, mpatel, was
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: notebook Keywords:
Cc: mvngu, chapoton Merged in:
Authors: Reviewers: Sébastien Labbé
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by ltw)

When uploading a zip file of worksheets to the notebook, if multiple worksheets have the same name, only one of the worksheets is extracted (multiple times). This is because the zip extraction code iterates through the file *names* instead of zipinfo file *objects* in the zip file. The attached patch takes care of this, and fixes uploads of zip files containing multiple worksheets having the same name.

This bit me several times today.

Apply:

  1. trac-10521-multiple-file-upload-v2.patch

Attachments (4)

trac-10521-multiple-file-upload.patch (1.7 KB) - added by jason 10 years ago.
apply to sagenb repository
trac-10521-multiple-file-upload-v2.patch (3.2 KB) - added by ltw 10 years ago.
apply to sagenb repo
download_worksheets.zip (1.0 KB) - added by ddrake 10 years ago.
test zip file with two worksheets with the same name
download_worksheets.2.zip (2.0 KB) - added by ddrake 10 years ago.
test zip file with two worksheets with the same name

Download all attachments as: .zip

Change History (21)

Changed 10 years ago by jason

apply to sagenb repository

comment:1 Changed 10 years ago by jason

  • Status changed from new to needs_review

comment:2 Changed 10 years ago by ddrake

This might conflict with #10652, which affects the same notebook code.

comment:3 Changed 10 years ago by jason

You're right; almost certainly there will be a conflict.

Changed 10 years ago by ltw

apply to sagenb repo

comment:4 Changed 10 years ago by ltw

  • Description modified (diff)

I have re-based the patch on sagenb-0.8.14 (which is the version that ships with sage-4.7).

comment:5 Changed 10 years ago by slabbe

  • Status changed from needs_review to needs_info

Can somebody upload a zip file somewhere so that we can more easily understand the problem and review this ticket.

For example, this is what I did while working on this issue of the new Sage flask Notebook.

Note that the file twist.py is deprecated in the the Flask Notebook. Hence, this patch will have to be re-based again. I am ready to do it since I worked in the exact same line while fixing the above issue. But a zip file would be appreciated. For example, I don't understand how two files can have the same name if they are in the same directory (I believe it is True, but I need an example to understand).

Sébastien

comment:6 Changed 10 years ago by ltw

I don't know anything about the new notebook, but here are the steps to reproduce the problem on the (unpatched) "old" notebook:

  1. Create two worksheets with different contents but give them the same name.
  2. Download all active worksheets via the link.
  3. (optional) Delete the worksheets from the server -- just to prevent confusion.
  4. Upload the zip file created in step 2.
  5. Examine the two worksheets created: they are duplicates of only one worksheet from the zip file. The other worksheet was never extracted.

Changed 10 years ago by ddrake

test zip file with two worksheets with the same name

Changed 10 years ago by ddrake

test zip file with two worksheets with the same name

comment:7 follow-up: Changed 10 years ago by ddrake

attachment:download_worksheets.2.zip is a zip file of the kind that ltw described. I created two worksheets with the same name but with different contents (either "This is the first worksheet" or "This is the second worksheet").

I made a mistake with attachment:download_worksheets.zip that is perhaps instructive: I opened the zipfile produced by the notebook (version 4.7) in Ubuntu's Archive Manager, and when I use "Save As" to save it to a file, I get the zip file you see in attachment:download_worksheets.zip.

So my guess is that either (1) the zip file spec doesn't really support having two files with the same name in the same directory, or (2) the zip tools being used don't properly handle these files. Since we can't control what zip tools users use, I propose that when the notebook creates the zip file, it fixes up the .sws filenames so that they are all unique. (That is, we fix this on the "creation end", and not on the "consumption end")

Sebastien, what did you do in the Flask notebook?

comment:8 in reply to: ↑ 7 Changed 10 years ago by slabbe

Sebastien, what did you do in the Flask notebook?

This :

http://code.google.com/r/slabqc-flask/source/detail?r=b8734085045a37e865fcdd9b28d954a02d0d5e76

(I just pushed it on the google code.)

to fix this issue :

http://code.google.com/p/sagenb/issues/detail?id=33

Sébastien

comment:9 Changed 10 years ago by jason

We need to fix this on the consumption end as well as the creation end, because as you say, we can't control what zip tools the users use. We don't know what kinds of zip files we'll be getting, and we don't know if those zip files have this problem (for example, an old zip file of worksheets produced by us).

comment:10 Changed 8 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:11 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:12 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:13 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:14 Changed 8 months ago by mkoeppe

  • Cc chapoton added
  • Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix
  • Status changed from needs_info to needs_review

Proposing to close all sagenb tickets as outdated, so that all remaining open tickets in the notebook component are about the Jupyter notebook.

comment:15 Changed 8 months ago by slabbe

  • Status changed from needs_review to positive_review

comment:16 Changed 8 months ago by mkoeppe

  • Authors Jason Grout deleted
  • Reviewers set to Sébastien Labbé

comment:17 Changed 8 months ago by chapoton

  • Resolution set to invalid
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.