Everything

php

Home » Project » Newsgroups » php.internals » xml_parser_create patch »

xml_parser_create patch

php.internals

Adam Dickmeiss

23 years ago

Hi,

I'd like to submit a small patch that allows xml_parser_create
and xml_parser_create_ns (Expat version) to honor the encoding of
an XML document (including UTF-16). This solves bug #23293. With this
patch xml_parser_create now works identical to the libxml2 version of it
(AFAIK).

Note that this patch _may_ break applications that assumes
ISO-8859-1 (or default encoding) for XML applications, but only
if the application omitted a desired encoding for xml_parser_create.

Applications that wish their XML parser to do decoding
but want a specific target encoding can use option
XML_OPTION_TARGET_ENCODING .

-- Adam

--
Adam Dickmeiss  mailto:adam@indexdata.dk  http://www.indexdata.dk
Index Data      T: +45 33410100           Mob.: 212 212 66

(Marcus Börger)

23 years ago

At 11:43 07.05.2003, Adam Dickmeiss wrote:

>[...]
>Note that this patch _may_ break applications that assumes
>ISO-8859-1 (or default encoding) for XML applications, but only
>if the application omitted a desired encoding for xml_parser_create.
>

Then why not make it the default value ?

[...]

marcus

Adam Dickmeiss

23 years ago

On Wed, May 07, 2003 at 11:59:24AM +0200, Marcus Börger wrote:

> At 11:43 07.05.2003, Adam Dickmeiss wrote:
> >[...]
> >Note that this patch _may_ break applications that assumes
> >ISO-8859-1 (or default encoding) for XML applications, but only
> >if the application omitted a desired encoding for xml_parser_create.
> >
>
> Then why not make it the default value ?

Then tell me . What would that default value be?

What I want a way to let the XML document tell my
Expat XML parser what encoding it's using..

-- Adam

>
> [...]
>
>
> marcus
>
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php

--
Adam Dickmeiss  mailto:adam@indexdata.dk  http://www.indexdata.dk
Index Data      T: +45 33410100           Mob.: 212 212 66

(Marcus Börger)

23 years ago

At 12:55 07.05.2003, Adam Dickmeiss wrote:

>On Wed, May 07, 2003 at 11:59:24AM +0200, Marcus Börger wrote:
> > At 11:43 07.05.2003, Adam Dickmeiss wrote:
> > >[...]
> > >Note that this patch _may_ break applications that assumes
> > >ISO-8859-1 (or default encoding) for XML applications, but only
> > >if the application omitted a desired encoding for xml_parser_create.
> > >
> >
> > Then why not make it the default value ?
>Then tell me . What would that default value be?
>
>What I want a way to let the XML document tell my
>Expat XML parser what encoding it's using..

Iirc then before the change every one supposes that ISO-8859-1
will be handled. So that should be the default. In other words we
should do BC.

marcus

Adam Dickmeiss

23 years ago

On Wed, May 07, 2003 at 03:02:17PM +0200, Marcus Börger wrote:

> At 12:55 07.05.2003, Adam Dickmeiss wrote:
> >On Wed, May 07, 2003 at 11:59:24AM +0200, Marcus Börger wrote:
> >> At 11:43 07.05.2003, Adam Dickmeiss wrote:
> >> >[...]
> >> >Note that this patch _may_ break applications that assumes
> >> >ISO-8859-1 (or default encoding) for XML applications, but only
> >> >if the application omitted a desired encoding for xml_parser_create.
> >> >
> >>
> >> Then why not make it the default value ?
> >Then tell me . What would that default value be?
> >
> >What I want a way to let the XML document tell my
> >Expat XML parser what encoding it's using..
>
>
> Iirc then before the change every one supposes that ISO-8859-1
> will be handled. So that should be the default. In other words we
> should do BC.

BC. Fine. I don't think you understand what I'm trying to
achieve with this fix.

Suppose I have an XML document that looks like this:

<?xml version="1.0" encoding="UTF-8"?>
<something/>

and I call xml_parser_create() without arguments, then
Expat (without my fix) will assume that the document
uses default encoding which currently is ISO-8859-1. So really,
there is nothing default about it. Since it ignores the
encoding in the XML document, it should probably be called
force_encoding (or similar:)

The only place where specifying an encoding makes sense is
if you parse XML files with no encoding attribute at all in them.
And frankly, even assuming ISO-8859-1 there, is bad. UTF-8 should be
default. But that's another issue.

Note that the issue is totally separate from target encoding
which is the desired encoding within PHP strings..

Now, there are probably apps out there uses this bad
behaviour, so BC is desirable. That's why I suggest
that using
xml_parser_create("");
does the _right_thing_. And other options are BC.
I am, of course, willing to send a patch for that.

-- Adam

> marcus

--
Adam Dickmeiss  mailto:adam@indexdata.dk  http://www.indexdata.dk
Index Data      T: +45 33410100           Mob.: 212 212 66

(Marcus Börger)

23 years ago

At 15:21 07.05.2003, Adam Dickmeiss wrote:

>On Wed, May 07, 2003 at 03:02:17PM +0200, Marcus Börger wrote:
> > At 12:55 07.05.2003, Adam Dickmeiss wrote:
> > >On Wed, May 07, 2003 at 11:59:24AM +0200, Marcus Börger wrote:
> > >> At 11:43 07.05.2003, Adam Dickmeiss wrote:
> > >> >[...]
> > >> >Note that this patch _may_ break applications that assumes
> > >> >ISO-8859-1 (or default encoding) for XML applications, but only
> > >> >if the application omitted a desired encoding for xml_parser_create.
> > >> >
> > >>
> > >> Then why not make it the default value ?
> > >Then tell me . What would that default value be?
> > >
> > >What I want a way to let the XML document tell my
> > >Expat XML parser what encoding it's using..
> >
> >
> > Iirc then before the change every one supposes that ISO-8859-1
> > will be handled. So that should be the default. In other words we
> > should do BC.
>BC. Fine. I don't think you understand what I'm trying to
>achieve with this fix.
>
>Suppose I have an XML document that looks like this:
>
>   <?xml version="1.0" encoding="UTF-8"?>
>   <something/>
>
>and I call xml_parser_create() without arguments, then
>Expat (without my fix) will assume that the document
>uses default encoding which currently is ISO-8859-1. So really,
>there is nothing default about it. Since it ignores the
>encoding in the XML document, it should probably be called
>force_encoding (or similar:)
>
>The only place where specifying an encoding makes sense is
>if you parse XML files with no encoding attribute at all in them.
>And frankly, even assuming ISO-8859-1 there, is bad. UTF-8 should be
>default. But that's another issue.

That was my point.
Sure it is a good thing to not longer ignore the document
but from perspective it is better to assume ISO-8859-1 if
no eccoding was specified in the doc since we did that before.

marcus

Rob Richards

23 years ago

Not sure if anyone grabbed this patch yet, so posting it.
If someone with karma to apache2handler would add it, be appreciated

http://www.ctindustries.net/sapi_apache2.diff

Fix for bug # 23455 / 21283 (memory leak)

Thanks,

Rob

Moriyoshi Koizumi

23 years ago

Your patch has jost been commited to both branches.

Thanks.

Moriyoshi

"Rob Richards" <rrichards@digarc.com> wrote:

Per Lundberg

23 years ago

On Wed, 2003-05-07 at 15:42, Marcus Börger wrote:

> Sure it is a good thing to not longer ignore the document
> but from perspective it is better to assume ISO-8859-1 if
> no eccoding was specified in the doc since we did that before.

What we did before was completely wrong.  It is not compatible with the
XML specification...  The default (if nothing specified) is UTF-8 or
UTF-16.

--
Best regards,

Per Lundberg / Capio ApS
Phone: +46-18-4186040
Fax: +46-18-4186049
Web: http://www.nobolt.com

Unnamed Person

23 years ago

-1 for the patch if it doesn't have a default value for the encoding.

We don't want to break BC again.

--
Regards.
M.CHAILLAN Nicolas
nicos@php.net
www.WorldAKT.com Hébergement de sites internets.
"Adam Dickmeiss" <adam@indexdata.dk> a écrit dans le message news:
20030507094303.GA7028@indexdata.com...

Adam Dickmeiss

23 years ago

On Wed, May 07, 2003 at 12:33:20PM +0200, nicos@php.net wrote:

> -1 for the patch if it doesn't have a default value for the encoding.
>
> We don't want to break BC again.

I respect that. I hope Streling does too with a new layer on
top ..

How about letting the empty string mean : Auto encoding??

xml_parser_create("");    //  any decoding! (pass 0 to Expat)

xml_parser_create();       // use default for input&output

xml_parser_create("x");    // Use decoding x for input&output

-- Adam

> --
> Regards.
> M.CHAILLAN Nicolas
> nicos@php.net
> www.WorldAKT.com Hébergement de sites internets.
> "Adam Dickmeiss" <adam@indexdata.dk> a écrit dans le message news:
> 20030507094303.GA7028@indexdata.com...
> > Hi,
> >
> > I'd like to submit a small patch that allows xml_parser_create
> > and xml_parser_create_ns (Expat version) to honor the encoding of
> > an XML document (including UTF-16). This solves bug #23293. With this
> > patch xml_parser_create now works identical to the libxml2 version of it
> > (AFAIK).
> >
> > Note that this patch _may_ break applications that assumes
> > ISO-8859-1 (or default encoding) for XML applications, but only
> > if the application omitted a desired encoding for xml_parser_create.
> >
> > Applications that wish their XML parser to do decoding
> > but want a specific target encoding can use option
> > XML_OPTION_TARGET_ENCODING .
> >
> > -- Adam
> >
> > --
> > Adam Dickmeiss  mailto:adam@indexdata.dk  http://www.indexdata.dk
> > Index Data      T: +45 33410100           Mob.: 212 212 66
> >
>
>
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php

--
Adam Dickmeiss  mailto:adam@indexdata.dk  http://www.indexdata.dk
Index Data      T: +45 33410100           Mob.: 212 212 66

Moriyoshi Koizumi

23 years ago

Adam Dickmeiss <adam@indexdata.dk> wrote:

> On Wed, May 07, 2003 at 12:33:20PM +0200, nicos@php.net wrote:
> > -1 for the patch if it doesn't have a default value for the encoding.
> >
> > We don't want to break BC again.
> I respect that. I hope Streling does too with a new layer on
> top ..
>
> How about letting the empty string mean : Auto encoding??
>
> xml_parser_create("");    //  any decoding! (pass 0 to Expat)
>
> xml_parser_create();       // use default for input&output
>
> xml_parser_create("x");    // Use decoding x for input&output
>

Looks like the best option to me.

Moriyoshi

Unnamed Person

23 years ago

Looks the best to me too.

Regards.
M.CHAILLAN Nicolas
nicos@php.net
www.WorldAKT.com Hébergement de sites internets.
----- Original Message -----
From: "Moriyoshi Koizumi" <moriyoshi@at.wakwak.com>
To: "Adam Dickmeiss" <adam@indexdata.dk>
Cc: <nicos@php.net>; <internals@lists.php.net>
Sent: Wednesday, May 07, 2003 3:54 PM
Subject: Re: [PHP-DEV] Re: xml_parser_create patch

Derick Rethans

23 years ago

On Wed, 7 May 2003, Adam Dickmeiss wrote:

> On Wed, May 07, 2003 at 12:33:20PM +0200, nicos@php.net wrote:
> > -1 for the patch if it doesn't have a default value for the encoding.
> >
> > We don't want to break BC again.
> I respect that. I hope Streling does too with a new layer on
> top ..
>
> How about letting the empty string mean : Auto encoding??
>
> xml_parser_create("");    //  any decoding! (pass 0 to Expat)

too much magic.

Derick

--
"my other box is your windows PC"
-------------------------------------------------------------------------
Derick Rethans                                 http://derickrethans.nl/
PHP Magazine - PHP Magazine for Professionals       http://php-mag.net/
-------------------------------------------------------------------------

Moriyoshi Koizumi

23 years ago

Derick Rethans <derick@php.net> wrote:

> On Wed, 7 May 2003, Adam Dickmeiss wrote:
>
> > On Wed, May 07, 2003 at 12:33:20PM +0200, nicos@php.net wrote:
> > > -1 for the patch if it doesn't have a default value for the encoding.
> > >
> > > We don't want to break BC again.
> > I respect that. I hope Streling does too with a new layer on
> > top ..
> >
> > How about letting the empty string mean : Auto encoding??
> >
> > xml_parser_create("");    //  any decoding! (pass 0 to Expat)
>
> too much magic.

I don't think so, as such a convension already exists in the
third parameter of htmlentities(). Are you going to come up with any other
solution?

Moriyoshi

Adam Dickmeiss

23 years ago

On Thu, May 08, 2003 at 09:19:08PM +0900, Moriyoshi Koizumi wrote:

> Derick Rethans <derick@php.net> wrote:
>
> > On Wed, 7 May 2003, Adam Dickmeiss wrote:
> >
> > > On Wed, May 07, 2003 at 12:33:20PM +0200, nicos@php.net wrote:
> > > > -1 for the patch if it doesn't have a default value for the encoding.
> > > >
> > > > We don't want to break BC again.
> > > I respect that. I hope Streling does too with a new layer on
> > > top ..
> > >
> > > How about letting the empty string mean : Auto encoding??
> > >
> > > xml_parser_create("");    //  any decoding! (pass 0 to Expat)
> >
> > too much magic.
>
> I don't think so, as such a convension already exists in the
> third parameter of htmlentities(). Are you going to come up with any other
> solution?

I don't think I understand what you're saying. Are you saying that
"" is OK to solve this issue? Or that "" is bad ? Or, (of course)
something else?

-- Adam

> Moriyoshi
>
> > Derick
> >
> > --
> >                                         "my other box is your windows PC"
> > -------------------------------------------------------------------------
> >  Derick Rethans                                 http://derickrethans.nl/
> >  PHP Magazine - PHP Magazine for Professionals       http://php-mag.net/
> > -------------------------------------------------------------------------
> >
> > --
> > PHP Internals - PHP Runtime Development Mailing List
> > To unsubscribe, visit: http://www.php.net/unsub.php
> >
>
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php

--
Adam Dickmeiss  mailto:adam@indexdata.dk  http://www.indexdata.dk
Index Data      T: +45 33410100           Mob.: 212 212 66

Moriyoshi Koizumi

23 years ago

Adam Dickmeiss <adam@indexdata.dk> wrote:
<snip>

> > > > How about letting the empty string mean : Auto encoding??
> > > >
> > > > xml_parser_create("");    //  any decoding! (pass 0 to Expat)
> > >
> > > too much magic.
> >
> > I don't think so, as such a convension already exists in the
> > third parameter of htmlentities(). Are you going to come up with any other
> > solution?
> I don't think I understand what you're saying. Are you saying that
> "" is OK to solve this issue? Or that "" is bad ? Or, (of course)
> something else?

?? I meant it to Derick...

Moriyoshi

Adam Dickmeiss

23 years ago

Hi,

here is another patch that recognizes "" as "proper
XML decoding". I also discovered incorrect string comparison
of other encodings in the xml_parser_create handler ..
Basically, a call such as
xml_parser_create("UT"); would be treated as
xml_parser_create("UTF-8");
.. which can't be right. It should trigger an error.

The patch only includes xml_parser_create not  _ns variant.
You people with karma can figure that out.

I have a character encoding handler for Expat that allows
expat to use conversion for all encodings it doesn't know.
I used it in another piece of software but it could be
easily modified for ext/xml. Now, since Expat is fading
away that might not be of interest.

-- Adam

On Thu, May 08, 2003 at 09:19:08PM +0900, Moriyoshi Koizumi wrote:

> Derick Rethans <derick@php.net> wrote:
>
> > On Wed, 7 May 2003, Adam Dickmeiss wrote:
> >
> > > On Wed, May 07, 2003 at 12:33:20PM +0200, nicos@php.net wrote:
> > > > -1 for the patch if it doesn't have a default value for the encoding.
> > > >
> > > > We don't want to break BC again.
> > > I respect that. I hope Streling does too with a new layer on
> > > top ..
> > >
> > > How about letting the empty string mean : Auto encoding??
> > >
> > > xml_parser_create("");    //  any decoding! (pass 0 to Expat)
> >
> > too much magic.
>
> I don't think so, as such a convension already exists in the
> third parameter of htmlentities(). Are you going to come up with any other
> solution?
>
> Moriyoshi
>
> > Derick
> >
> > --
> >                                         "my other box is your windows PC"
> > -------------------------------------------------------------------------
> >  Derick Rethans                                 http://derickrethans.nl/
> >  PHP Magazine - PHP Magazine for Professionals       http://php-mag.net/
> > -------------------------------------------------------------------------
> >
> > --
> > PHP Internals - PHP Runtime Development Mailing List
> > To unsubscribe, visit: http://www.php.net/unsub.php
> >

--
Adam Dickmeiss  mailto:adam@indexdata.dk  http://www.indexdata.dk
Index Data      T: +45 33410100           Mob.: 212 212 66

Unnamed Person

23 years ago

Okay I agree that using UT and not UTF-8 is bad, but isn't it a BC break if
people were using that to get UTF-8?

Regards.
M.CHAILLAN Nicolas
nicos@php.net
www.WorldAKT.com Hébergement de sites internets.
----- Original Message -----
From: "Adam Dickmeiss" <adam@indexdata.dk>
To: "Moriyoshi Koizumi" <moriyoshi@at.wakwak.com>
Cc: "Derick Rethans" <derick@php.net>; "Adam Dickmeiss" <adam@indexdata.dk>;
<nicos@php.net>; <internals@lists.php.net>
Sent: Friday, May 09, 2003 4:11 PM
Subject: Re: [PHP-DEV] Re: xml_parser_create patch

> Hi,
>
> here is another patch that recognizes "" as "proper
> XML decoding". I also discovered incorrect string comparison
> of other encodings in the xml_parser_create handler ..
> Basically, a call such as
>  xml_parser_create("UT"); would be treated as
>  xml_parser_create("UTF-8");
> ... which can't be right. It should trigger an error.
>
> The patch only includes xml_parser_create not  _ns variant.
> You people with karma can figure that out.
>
> I have a character encoding handler for Expat that allows
> expat to use conversion for all encodings it doesn't know.
> I used it in another piece of software but it could be
> easily modified for ext/xml. Now, since Expat is fading
> away that might not be of interest.
>
> -- Adam
>
> On Thu, May 08, 2003 at 09:19:08PM +0900, Moriyoshi Koizumi wrote:
> > Derick Rethans <derick@php.net> wrote:
> >
> > > On Wed, 7 May 2003, Adam Dickmeiss wrote:
> > >
> > > > On Wed, May 07, 2003 at 12:33:20PM +0200, nicos@php.net wrote:
> > > > > -1 for the patch if it doesn't have a default value for the

encoding.

> > > > >
> > > > > We don't want to break BC again.
> > > > I respect that. I hope Streling does too with a new layer on
> > > > top ..
> > > >
> > > > How about letting the empty string mean : Auto encoding??
> > > >
> > > > xml_parser_create("");    //  any decoding! (pass 0 to Expat)
> > >
> > > too much magic.
> >
> > I don't think so, as such a convension already exists in the
> > third parameter of htmlentities(). Are you going to come up with any

other

> > solution?
> >
> > Moriyoshi
> >
> > > Derick
> > >
> > > --
> > >                                         "my other box is your windows

PC"

> >
> -------------------------------------------------------------------------
> > >  Derick Rethans

http://derickrethans.nl/

> > >  PHP Magazine - PHP Magazine for Professionals

http://php-mag.net/

Moriyoshi Koizumi

23 years ago

<nicos@php.net> wrote:

> Okay I agree that using UT and not UTF-8 is bad, but isn't it a BC break if
> people were using that to get UTF-8?

"UT" has never been a fully qualified encoding name in the first place,
and if an user uses it for UTF-8, it's simply the user's mistake.
No BC break then.

Moriyoshi

Moriyoshi Koizumi

23 years ago

I'm going to apply a modified version of your patch soon.

Moriyoshi

Adam Dickmeiss <adam@indexdata.dk> wrote: