search

KSP-CKAN / CKAN

The Comprehensive Kerbal Archive Network
https://forum.kerbalspaceprogram.com/index.php?/topic/197082-*
Other
1.97k stars 347 forks source link

Jenkins does not approve of "&" in KS names. #816

Closed Dazpoet closed 9 years ago

Dazpoet commented 9 years ago

For tracking purposes I'm opening this issuereport. Which is really about netkan.exe but I found it through Jenkins so well... That's why it's named weirdly.

It seems Jenkins does not approve of certain signs and url-encode multiple times while building. This happens in https://github.com/KSP-CKAN/NetKAN/pull/939 where Jenkins fail due to re-url-encode of the & between storing the download in cache and then searching the cache for said download.

Jenkins (who I assume run on linux) changes %26 -> %2526. Under OS X on my local mac netkan.exe failes in a similar way but seems to re-url-encode an additional time between storing and checking cache. Under OS X it goes %26 -> %252526 and I can't see any step where the first re-encode happen while using the --debug flag.

Dazpoet commented 9 years ago

Well would you look at that. I found another mod with the same problem here.

I notice that after Jenkins was updated today he seems to be giving more information so from the log linked in this post we see that

1509 [1] DEBUG CKAN.NetKAN.KSAPI (null) - Expanding /mod/688/Bacon%20Labs:%20Stockalike%20Ariane%20%26%20More/download/0.2.2 to full KS URL
1515 [1] DEBUG CKAN.NetKAN.KSAPI (null) - Expanded URL is https://kerbalstuff.com/mod/688/Bacon%20Labs:%20Stockalike%20Ariane%20%2526%20More/download/0.2.2

Expanding the mod-URL seems to create the first step %26->%2526

1597 [1] DEBUG CKAN.Net (null) - Downloading https://kerbalstuff.com/mod/688/Bacon%20Labs:%20Stockalike%20Ariane%20%2526%20More/download/0.2.2 to /tmp/CdFileMgr/db7d0ba7-5f2a-48.tmp
2278 [1] INFO CKAN.Net (null) - Download failed, trying with curlsharp...
4119 [1] DEBUG CKAN.Net (null) - curlsharp download successful
4123 [1] DEBUG CKAN.NetFileCache (null) - Storing https://kerbalstuff.com/mod/688/Bacon Labs: Stockalike Ariane %2526 More/download/0.2.2
4124 [1] DEBUG CKAN.NetFileCache (null) - Checking cache for https://kerbalstuff.com/mod/688/Bacon Labs: Stockalike Ariane %2526 More/download/0.2.2
4125 [1] DEBUG CKAN.NetFileCache (null) - Storing /tmp/CdFileMgr/db7d0ba7-5f2a-48.tmp in dummy_ksp/CKAN/downloads/FED72E80-BaconLabs-0.2.2.zip

Downloading, storing and caching the file all seem to resolve with %2526 but then

4157 [1] DEBUG CKAN.NetKAN.KSAPI (null) - Expanding /mod/688/Bacon Labs: Stockalike Ariane & More to full KS URL
4160 [1] DEBUG CKAN.NetKAN.KSAPI (null) - Expanded URL is https://kerbalstuff.com/mod/688/Bacon%20Labs:%20Stockalike%20Ariane%20&%20More
4183 [1] DEBUG CKAN.CkanModule (null) - In-client JSON schema validation unimplemented.
4237 [1] DEBUG CKAN.NetFileCache (null) - Checking cache for https://kerbalstuff.com/mod/688/Bacon Labs: Stockalike Ariane %252526 More/download/0.2.2
4239 [1] FATAL CKAN.NetKAN.MainClass (null) - Error: Unable to find BaconLabs in the cache

And we get a fail because in this stage it goes %2526->%252526. At timestamp 4160 it oddly seems that Jenkins just grabs the "&" sign directly, something that doesn't happen anywhere else.

Dazpoet commented 9 years ago

After the updates to Jenkins today we now see the same behaviour for KSP-CKAN/NetKAN#939. Relevant parts of log below

1765 [1] DEBUG CKAN.NetKAN.KSAPI (null) - Expanding /mod/79/Salyut%20Stations%20%26%20Soyuz%20Ferries/download/0.92 to full KS URL
1780 [1] DEBUG CKAN.NetKAN.KSAPI (null) - Expanded URL is https://kerbalstuff.com/mod/79/Salyut%20Stations%20%2526%20Soyuz%20Ferries/download/0.92
...
1893 [1] DEBUG CKAN.NetKAN.MainClass (null) - Mod: Salyut Stations & Soyuz Ferries 0.92
1896 [1] DEBUG CKAN.NetKAN.KSVersion (null) - Downloading https://kerbalstuff.com/mod/79/Salyut Stations %2526 Soyuz Ferries/download/0.92
...
11270 [1] DEBUG CKAN.NetFileCache (null) - Storing https://kerbalstuff.com/mod/79/Salyut Stations %2526 Soyuz Ferries/download/0.92
11271 [1] DEBUG CKAN.NetFileCache (null) - Checking cache for https://kerbalstuff.com/mod/79/Salyut Stations %2526 Soyuz Ferries/download/0.92
11273 [1] DEBUG CKAN.NetFileCache (null) - Storing /tmp/CdFileMgr/b878dbeb-a10a-4a.tmp in dummy_ksp/CKAN/downloads/1D4C656E-RN-Salyut-0.92.zip
...
11318 [1] DEBUG CKAN.NetKAN.KSAPI (null) - Expanded URL is https://kerbalstuff.com/mod/79/Salyut%20Stations%20&%20Soyuz%20Ferries
11342 [1] DEBUG CKAN.CkanModule (null) - In-client JSON schema validation unimplemented.
11397 [1] DEBUG CKAN.NetFileCache (null) - Checking cache for https://kerbalstuff.com/mod/79/Salyut Stations %252526 Soyuz Ferries/download/0.92

Just like with the above example it seems Jenkins use %2526 until it gets to timestamp 11318 where it suddenly jumps to using & and then that somehow incorrectly becomes %252526 at timestamp 11397.

One "positive" thing is that we now see the same error under OS X and linux.

Dazpoet commented 9 years ago

Aaand another one. https://github.com/KSP-CKAN/NetKAN/pull/1003

Dazpoet commented 9 years ago

Copied from https://github.com/KSP-CKAN/NetKAN/pull/992

Basically KSP-CKAN/CKAN#816 needs some testing. This file is very simple so I'll merge it based on manual inspection and see if something explodes.

Motivation

Tantares was updated by the netkan-bot 16 days ago so "&" in names worked then. @BasharMilesTeg opened https://github.com/KSP-CKAN/NetKAN/pull/939 10 days ago which didn't work. If merging of this mod doesn't work we've narrowed it down to a 6 day gap to look for changes. Sounds a lot better than shooting in the dark. At worst we get a broken .netkan file which can easily be removed and maybe someone will have to kick the netkan-bot again.

So BaconLabs didn't work and as such the problem is now narrowed down to changes between Tantares last update and the creation of the PR for mods by RaiderNick.

pjf commented 9 years ago

On this now.

pjf commented 9 years ago

On my system, which uses mono 3.2.8, everything works fine. Jenkins uses a later version, and doesn't work, even with exactly the same .exe file, so I'm guessing the underlying libraries are different. I'm still tracking this down. :P

pjf commented 9 years ago

It looks like the source of this bug is in mono's .ToString() method for URLs. The docs say that everything should come back unescaped except for #, ? and %. However it looks like the mono implementation is not unescaping ampersands.

Since our code assumes that .ToString() will return an unescaped string, bad things happen when we then later to try escape it, and end up with the double-escaped % character. The fact that we're seeing this on jenkins as well means that this may a long-term bug in mono, or I'm completely misunderstanding the .NET spec.

I don't feel like submitting patches to mono today, so I'll be looking at a work-around for the netkan client.