Bug 191499

Summary:	HTTP(S) URL escaping broken (URL::Parse makes path eat query)
Product:	[openSUSE] SUSE Linux 10.1	Reporter:	Uwe Gansert <ug>
Component:	YaST2	Assignee:	Josef Reidinger <jreidinger>
Status:	RESOLVED FIXED	QA Contact:	Stanislav Visnovsky <visnov>
Severity:	Normal
Priority:	P5 - None	CC:	jreidinger, suse-beta
Version:	Final
Target Milestone:	---
Hardware:	Other
OS:	Other
Whiteboard:
Found By:	Other	Services Priority:
Business Priority:		Blocker:	---
Marketing QA Status:	---	IT Deployment:	---
Attachments:	patch for URL.ycp

Description Uwe Gansert 2006-07-11 09:11:12 UTC

with autoyast people use URLs like this:
http://1.2.3.4/gen_xml.cgi?mac=...
those URLs are escaped since 10.1 and the parameter list gets destroyed.

I have attached a patch proposal. Better check if I overlooked something.

For example:
if (tokens["path"]:"" != "" && find (tokens["path"]:"", "/") != 0 )
in the original code looked broken to me anyway. The "find" condition is only 0 if the "/" is on position 0. If there is no "/" at all or on any other position in the string, the find is true. Is that really wanted that way? I did not dig very deep to find it out.

To explain my patch, I escape nothing after the last "?" in HTTP/HTTPS requests.
I hope that's okay.

Comment 1 Uwe Gansert 2006-07-11 09:11:46 UTC

Created attachment 93143 [details]
patch for URL.ycp

Comment 2 Martin Vidner 2006-07-11 11:49:03 UTC

Please give a better example: what input to URL::Build produces different behavior in 10.0 and 10.1?
Ideally write a test case in https://svn.suse.de/svn/yast/trunk/yast2/library/types/testsuite/tests/URL.ycp :)

Comment 3 Uwe Gansert 2006-07-11 11:59:08 UTC

on 10.0:
http://server/classes/autoinst.xml?bla=blupp

on 10.1 it's escaped to:
http://server/classes/autoinst.xml%3fbla%3dblupp

Comment 4 Martin Vidner 2006-07-11 12:45:01 UTC

bug 179913 may be related.

[YCP] autoinstall/io.ycp:67 UWE $["fragment":"", "host":"10.10.0.162", "pass":"", "path":"/classes/autoinst.xml?bla=blupp", "port":"", "query":"", "scheme":"http", "user":""]
you patched Build but the problem is actually in Parse. "path" eats the "query".

Comment 5 Martin Vidner 2006-07-11 12:55:28 UTC

Originally reported on SLES 10 RC3.
Moving to make it public.

Comment 6 Martin Vidner 2006-11-23 17:55:27 UTC

As URL processing is hard, we should take advantage of existing code in libzypp and make Pkg:: bindings for its URL routines.

Comment 8 Andreas Jaeger 2008-10-23 19:34:21 UTC

can this be marked as fixed?

Comment 9 Josef Reidinger 2011-06-20 12:55:34 UTC

(In reply to comment #1)
> Created an attachment (id=93143) [details]
> patch for URL.ycp

I check patch and it is wrong, queries should be in tokens["query"]. So it looks more like bad usage or bug in parse method. 
Uwe - how do you parse it and where it is used, so I can test it?

Comment 10 Uwe Gansert 2011-06-20 13:10:46 UTC

it's used in multiple places in autoyast:

ug@taylor:/space/YaST_trunk/trunk/autoinstallation/src> svngrep URL::
./include/io.ycp:        string full_url = URL::Build(toks);
./include/io.ycp:        AutoinstConfig::urltok=URL::Parse(url);
./modules/AutoinstScripts.ycp:      map tok = URL::Parse(newloc);
./modules/AutoinstScripts.ycp:      map t = URL::Parse(s["location"]:"");
./modules/AutoinstScripts.ycp:      map toks = URL::Parse(s["location"]:"");
./modules/AutoinstConfig.ycp:       result = URL::Parse (cmdLine);
./modules/AutoinstFile.ycp:         map tok = URL::Parse(newloc);
./modules/AutoinstImage.ycp:        urltok = URL::Parse (AutoinstSoftware::image["script_location"]:"");

it's used for parsing the autoyast=.... parameter too
so to test it, simply start an autoinstallation by passing autoyast=... to linuxrc

Comment 11 Josef Reidinger 2011-06-20 14:17:32 UTC

OK, I try it and it works for me for SLE11SP2 alpha2. Could you please verify that it works and if not, could you please post exact parameter you use?
thanks

Comment 12 Josef Reidinger 2011-06-20 14:22:49 UTC

Looks like fixed. Feel free to reopen if it appear again.