I'm rereading rfc2396 to verify or change the contents of the escape matrix in nsStdEscape.
changes: @ now also allowed in directory ' now allowed everywhere ~ now allowed everywhere ; now also allowed in scheme, username, password and host this needs some more testing of course ...
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla0.9.7
Some more explanations: ' and ~ Both characters belong into the unreserverd charset (section 2.3). No restrictions, so no escaping should be necessary at all. @ and ; Both characters belong to the reserved charset. According to section 3.2 ";" is reserved in the authority component for registry-based authoritys and can be used in server-based authoritys. "@" is not reserved in the directory component according to section 3.3, but it is also explicitly allowed, so we can remove the current escaping there.
maybe the best way to go about getting this in the builds is to conditionally use this new matrix based on a preference or env var. andreas, are you up for adding the code which will allow QA to flip a switch and use this new matrix?
Sure, I will try that. Having two matrixes is a little bloat, but okay I think. I would like to use a pref to switch between the two matrixes, how much would that hurt performance?
just check the pref once at startup and cache the result, or if you want to get fancy, add a pref change callback :)
Hmmm ... for now I have it working with a pref check at each nsStdEscape call and it works fine, but makes xpcom requires pref. Looking into the caching stuff ...
actually, an environment variable would be better. no ones going to approve making xpcom depend on libpref BTW.
That's what I thought :(
Created attachment 60032 [details] [diff] [review] switch between two escape matrix based on env-variable ALT_ESC_MATRIX switch between the matrixes based on env variable ALT_ESC_MATRIX. No caching yet, can't find a suitable place to store the global variable.
Attachment #56592 - Attachment is obsolete: true
andreas: i think you only check the env variable once... calling getenv everytime is unnecessary.
Yes I know, but I can't find a suitable place to cache the env variable. On the other hand I don't think it is necessary to do this env-stuff at all. ' and ~ are not used in the urlparser anywhere, changing it's escaping value is without any danger. We look at @ in ParseURL and ParseAuthority, in ParseURL we first look for /, so there is no danger mistaking a @ as part of the path with the @ inside the authority. No danger here too. ; in scheme, username, password and host is a bit more complicated. There could be a problem with the urlparser. I have to dig deeper there. So what about dropping the changes for ; and put the other changes in (unconditionally) after the tree opens for 0.9.8?
sounds reasonable to me.
Created attachment 60223 [details] [diff] [review] new patch, direct change to escape matrix, but only for @'~
Doug, I moved the ; part (which could have caused problems) out of the patch, the other changes are trivial without any impact I can think of to url parsing. So, I think we can put this directly into the matrix without any pref or env variable.
Comment on attachment 60223 [details] [diff] [review] new patch, direct change to escape matrix, but only for @'~ sr=darin
fix checked in, dropped the ; stuff
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.