Re: Re: cvs: pear /Structures_DataGrid/DataGrid/Renderer HTMLTable.php

On 3/3/06, Olivier Guilyardi <ojaiml@nerim.net> wrote: > Justin Patrin wrote: > > On 3/3/06, Olivier Guilyardi <ojaiml@nerim.net> wrote: > > > >>>>+ * Note: users who want their GET parameters separated by > >>>>+ * "&amp;" instead of "&" (see Bug > >>>>#6151) should properly > >>>>+ * configure the "arg_separator.output" php ini > >>>>setting */ > >>>> $url .= http_build_query(array_merge($common, $get)); > >>> > >>>I don't think that this is right. http_build_query could be used for > >>>non-HTML output within the same script and changing the php.ini > >> > >>Agreed. Everything would be simpler if http_build_query() accepted an additional > >>$separator argument. > > > > Yes. > > > >>>IMHO this option is akin to the infamous magic_quotes_gpc and should > >>>be worked around in the same manner. It can be used as the same kind > >>>of premature escaping mechanism. > >> > >>Do we have to provide a "workaround" or is this simply a PHP bug ? Why to > >>provide workarounds for what can be fixed at its root ? > > > > magic_quotes_gpc is also IMHO a PHP bug. Along with overloading. But > > that's another conversation. > > There's a big difference between magic_quotes_gpc and http_build_query(). The > former is a very old PHP design choice, but the later is a pretty recent > feature. So IMO it is still time to post a PHP bug about http_build_query() > > > I would say that either you shouldn't use this function due to > > possible inconsistencies or check the option and run htmlentities if > > the option isn't set right. > > Actually, a solution is to paste the PHP/Compat/Function/http_build_query..php > content into new methods inside the HTMLTable driver. That's what's done in > Pager if IIRC. There I could simply use any separator I like. But I'm not going > to do this. Too many lines for a such little workaround. > > Now, as you said, by checking what the content of arg_separator.output is before > calling htmlentities I would avoid the "&amp;amp;" problem. This would be a > quite short workaround, I must confess. > > Actually, htmlentities even seems to be what PHP maintainers recommend... > http://bugs.php.net/bug.php?id=30049 > > > My point is really that it's the job of the output code to make sure > > that the HTML is escaped correctly, it shouldn't depend on the user > > altering an option. > > Agreed, but PHP options are not so obscure, they are parameters that a lot of > people can change to configure their scripts. But some can't, that's true.. > > > But then again, maybe I'm completely off-base here. You're welcome to > > leave it however you want... > > No, you're quite right about the problem... Okay, so I'm going to implement the > htmlentities() workaround. > :-) Sounds good. > Do you think that a new PHP bug makes any sense ? > IMHO it would make sense to have that option removed entirely. Then again, I doubt that the PHP maintainers would want to do that.. -- Justin Patrin