Bugs when downloading a folder as zip

[ikarus23]

Hi,

first of all: thanks for the great tool!

I've noticed two bugs when downloading a folder as zip:

• The first letter of the folder is missing: A folder named foobar will be oobar inside the zip.
• The file extension dot is missing: A file foobar.py will be foobarpy inside the zip.

[patrickhener]

Hi Mr. G 😂,

thanks for reporting. As soon as I have some spare minutes I will try and debug. This Was working once and broke some time on the way. Didn't recognise this yet. So again thanks for telling.

See ya around.

[patrickhener]

Hi there,

can you provide further details @ikarus23 ? On what OS are you on? I recreated just now in Arch Linux and it is working fine for me.

Maybe the last commit changed the problem as well 🤷 . So you might as well pull current main and build from source and see if the problem is resolved?

[ikarus23]

Hi Patrick. I use Arch, btw., too :) I've used the latest master (without the recent patch). One thing that might be different: I downloaded and extracted the zip file on a Windows 10 machine with default on-board tools (edge/explorer).

I probably can do more testing on the weekend.

[patrickhener]

Hi, interesting. I just now reproduced. I started on arch linux within a folder.

> tree -L 2
.
├── foobar
│   └── test.py
└── test.go

1 directory, 2 files

Then going to Windows I browsed there and downloaded the complete folder (mark all) and only the folder foobar (tick only foobar) with chrome and edge. I extracted all four zip files with onboard "Alles extrahieren".

It resulted in correct folder and file names. So basically I cannot reproduce. Please pull current main branch and do:

make build

Then run goshs from binary result in folder dist/linux_amd64. Let me know if the issue persists. If not I can tag a new version and release (meaning the previous change might have fixed your issue as well).

[ikarus23]

I can reproduce it. Maybe we are doing it differently. What I do is:

1. have a folder with more folders

tree test 
test
├── foobar
│   ├── blubb
│   ├── test-foo.py
│   └── test.py
├── test1
└── test2

2. Start goshs -d test
3. Browse to the share
4. Select the foobar folder via the checkbox before it
5. Press the "Donload selected" button at the end of the file listing
6. Extract the 1647004714_goshs_download.zip with WIndows

For a git pull and make build I get:

[OK] Cleaned up!
[*] Minifying and compiling scss and js
make: uglifyjs: No such file or directory
make: *** [Makefile:6: generate] Error 127

[patrickhener]

Ok I just now reproduced again exactly like you did. My results are:

Folder foobar is extracted correctly and named "foobar".
but the content within the folder are the files ".py" instead of "test.py". There is a file "-foo.py" which is supposed to be "test-foo.py" and file "blubb" is untouched.

And this is really strange cause this is a different result as you have right? But still not correct. I don't get it tbh.

So other test case is to browse into the folder called test and just run goshs without -d argument. Doing it this way it perfectly works. Although this is just a workaround it makes it work on my side. Can you please reproduce and see if this behaves the same for you. If so I might need to look into what -d breaks when zipping.

Running it in the folder test with -d . to point it to the pwd will result in having a folder called "oobar" and the files are "blubb", "test-foopy" and "testpy". So I think it is related to -d

[patrickhener]

I think I narrowed it down. Did some debug output at the relevant lines:

		// filepath is fs.Webroot + file relative path
		// this would result in a lot of nested folders
		// so we are stripping fs.Webroot again from the structure of the zip file
		// Leaving us with the relative path of the file
		zippath := strings.ReplaceAll(filepath, fs.Webroot, "")
		fmt.Printf("Zip path will be '%s'\n", zippath)
		f, err := resultZip.Create(zippath[1:])
		if err != nil {
			return err
		}

So two test show:

> pwd
/home/patrick
> goshs -d /tmp/test
INFO   [2022-03-11 14:56:46] Serving on interface lo bound to 127.0.0.1:8000 
INFO   [2022-03-11 14:56:46] Serving on interface eth0 bound to 192.168.1.122:8000 
INFO   [2022-03-11 14:56:46] Serving on interface virbr0 bound to 192.168.122.1:8000 
INFO   [2022-03-11 14:56:46] Serving on interface vmnet1 bound to 192.168.17.1:8000 
INFO   [2022-03-11 14:56:46] Serving on interface vmnet8 bound to 192.168.50.1:8000 
INFO   [2022-03-11 14:56:46] Serving HTTP from /tmp/test                  
INFO   [2022-03-11 14:56:50] 192.168.122.215:49942 - - "GET / HTTP/1.1" - 200 
Files after cleaning
[/foobar]
Starting new walker '/tmp/test/foobar'
Trying to add file '/tmp/test/foobar' to zip
Trying to add file '/tmp/test/foobar/blubb' to zip
Zip path will be '/foobar/blubb'
Trying to add file '/tmp/test/foobar/test-foo.py' to zip
Zip path will be '/foobar/test-foo.py'
Trying to add file '/tmp/test/foobar/test.py' to zip
Zip path will be '/foobar/test.py'

The above is working.

vs

> pwd
/tmp
> goshs -d test
INFO   [2022-03-11 14:57:19] Serving on interface lo bound to 127.0.0.1:8000 
INFO   [2022-03-11 14:57:19] Serving on interface eth0 bound to 192.168.1.122:8000 
INFO   [2022-03-11 14:57:19] Serving on interface virbr0 bound to 192.168.122.1:8000 
INFO   [2022-03-11 14:57:19] Serving on interface vmnet1 bound to 192.168.17.1:8000 
INFO   [2022-03-11 14:57:19] Serving on interface vmnet8 bound to 192.168.50.1:8000 
INFO   [2022-03-11 14:57:19] Serving HTTP from test                       
INFO   [2022-03-11 14:57:21] 192.168.122.215:49948 - - "GET / HTTP/1.1" - 200 
Files after cleaning
[/foobar]
Starting new walker 'test/foobar'
Trying to add file 'test/foobar' to zip
Trying to add file 'test/foobar/blubb' to zip
Zip path will be '/foobar/blubb'
Trying to add file 'test/foobar/test-foo.py' to zip
Zip path will be '/foobar/-foo.py'
Trying to add file 'test/foobar/test.py' to zip
Zip path will be '/foobar/.py'

And this is because of strings.ReplaceAll. The parameter fs.Webroot will be what is handed over by -d. If you choose to do -d test it will strip test from every filename resulting in the wrong filenames after unzipping. If you provide absolute path with -d you will have the expected behaviour.

Imma try to fix it now I know what is wrong.

[ikarus23]

Interesting. But for me it is especially the . and the first letter of the folder:

[patrickhener]

Yeah because default for -d is also . and then it gets stripped out as well. I fixed that on my side by constructing a proper webroot as a absolute path when initializing the fileserver. So see attached a dev build of goshs and please verify that this will work for you. If so I am pushing a new tag then.
goshs.tar.gz

Those are the code changes to main.go in function init()

	// Abspath for webroot
	var err error
	mylog.Debugf("Webroot before transformation: %s", webroot)
	if !filepath.IsAbs(webroot) {
		webroot, err = filepath.Abs(filepath.Join(wd, webroot))
		if err != nil {
			mylog.Fatalf("Webroot cannot be constructed: %+v", err)
		}
	}
	mylog.Debugf("Final webroot is: %s", webroot)

If you start goshs with DEBUG=TRUE ./goshs -d <argument> you can see the transformation

[ikarus23]

I will retry everything with the latest pre-build release. I'm guessing I might messed up there.

[ikarus23]

You are right! It is because of -d .!

[patrickhener]

Yeah that messed up the webroot var and then gets stripped later when processing the files for the zipping. Code snip above circumvents this by handing webroot to FileServer as absolute path instead of relative to begin with.

[ikarus23]

Yeees! Your fix works! Thank you!

[patrickhener]

Okay thanks for verifying. Give me another 10 minutes and I'll push version v0.1.8. Then please allow github about another 30 minutes to be able to process it is the latest tagged release. So in about an hour I assume a go install github.com/patrickhener/goshs@latest will give you the fixed version than.

Thanks again Gerhard for using and testing and more over reporting the bug instead of just dumping the tool.

Best regards :D