283930 - Use CGI::Util::unescape instead of url_decode

Status: RESOLVED WONTFIX

(Bugzilla :: Bugzilla-General, enhancement, P2)

Description

[Max Kanat-Alexander]

This is actually only used by query.cgi.

Really, instead of using this function, query.cgi's PrefillForm should just use
the CGI stuff, and then we should eliminate this function.

But for now, we'll just move it.

Comment 1

[Max Kanat-Alexander]

Actually, CGI already provides a method that does this for us. Why write our own
method when everybody already has this method?

Comment 2

[Max Kanat-Alexander]

Attachment: Change url_decode to unescape

Another tiny, tiny, simple, simple patch.

CGI.pl is pretty easy to hack away at, so far.

Comment 3

[Marc Schumann [:Wurblzap]]

Comment on attachment 175705 [details] [diff] [review]
Change url_decode to unescape

I think I'm not up to that review. While it's certainly working, and the CGI.pm
code has improved since the time of stealing, I'm hesitant to use internal
utilities marked as "no public subroutines"... The original reasons for
choosing stealing over using may still apply.

Opening r?.

Comment 4

[Max Kanat-Alexander]

Comment on attachment 175705 [details] [diff] [review]
Change url_decode to unescape

OK, I'll bet justdave knows why we originally changed...

Comment 5

[Max Kanat-Alexander]

Comment on attachment 175705 [details] [diff] [review]
Change url_decode to unescape

OK, maybe glob will know...?  :-) He's listed as our CGI.pm guy. :-)

Comment 6

[:glob ✱]

like mark i'm in two minds about this one.

on the negative side CGI::Util::unescape() is documented as an internal
non-public routine, and it would be bad form to use it.  i suspect this is the
reason why the code was copied into CGI.pl.

on the other hand, CGI's unescape adds utf8 support, which we'll eventually require.

Comment 7

[Dave Miller [:justdave]]

The existing Bugzilla stuff was swiped from CGI.pm before my time.

See URI::Escape for a public interface to such a routine.  (If that's the only
place we're using it, and we haven't already picked up the URI suite via some
other dependency, we might as well continue using our own version)

Comment 8

[Max Kanat-Alexander]

Yeah. XML::Parser requires URI, but I can't see anywhere else that we've brought
it in in an actually *required* module.

And we don't want to use undocumented routines in CGI::Util, definitely, now
that I know they're undocumented.