hi,

Hi,

now that we have a publicly available alpha on Windows we get the first
feedback from Windows users. Within quite short time we got two bug
reports (#45992 #45994) about a change in the ini-parsing:

With <=5.2 we allow

include_path = "c:\foo\bar"

Pierre completely disagrees, I guess he'll answer to this mail and tell
why :-)

Things is that we have expandable variables now (or env). I think that
having double quotes working like in php is a good and safe thing. It
could be too tricky (or pointless) to have to deal with two kind of
backslashes, depending on the context. Especially as (and Johannes
should have mentioned it in his mail) the fix is as easy than:

include_path=c:\foo\bar\

or

include_path="c:/foo/bar/"

Cheers,

Pierre

http://blog.thepimp.net | http://www.libgd.org

Hi!

Even if it may mean drop the new ini parser and features?

Which features require \ to be special character? Maybe we could change
them so they do not?

--
Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com

Pierre Joye kirjoitti:

hi,

but now it's too late for such changes,
we have to put the old behavior back.

Even if it may mean drop the new ini parser and features?

I'm waiting Jani's anwer on that.

"" allow using ini variables, that's basically with " the only that has to
work.. just ignore other escaping?

--Jani

Stan Vassilev | FM wrote:

I have to agree, this kind of change would be very frustrating for
Win32 users, even though the solution is that simple.

Nobody reads update guides or installation instructions, they're used
to write path like that and they'll continue to do it, so we'll have
much more bug reports after the release if we keep it this way. Just
try to imagine how much PHP installations will suddenly stop working
after the upgrade.
I do agree that this syntax is actually correct and it should have
been done this way since the very beginning, but now it's too late for
such changes, we have to put the old behavior back.

I'm a Windows PHP user. It's never late for a change of that kind.
Upgrading your PHP installation is officially a Big Event, and tweakin
the ini is a one-time operation per upgrade, so I believe some people
may be overestimating the pain it will cause.

+1

There won't be any bug reports, if the ini parser detects improper
backslash usage (i.e., using a backslash with a following character
which doesn't form correct escape combination) and stops with
informative and descriptive message explaining the parser changes. Then
anyone who can read, will figure it out, and fix it.

"Nobody reads update guides or installation instructions"

...until it breaks :) It can be spelled out clearly enough. The user just
wants clear instruction or an easy hit on google.

Stan Vassilev | FM kirjoitti:

I have to agree, this kind of change would be very frustrating for
Win32 users, even though the solution is that simple.

Nobody reads update guides or installation instructions, they're used
to write path like that and they'll continue to do it, so we'll have
much more bug reports after the release if we keep it this way. Just
try to imagine how much PHP installations will suddenly stop working
after the upgrade.
I do agree that this syntax is actually correct and it should have
been done this way since the very beginning, but now it's too late for
such changes, we have to put the old behavior back.

I'm a Windows PHP user. It's never late for a change of that kind.
Upgrading your PHP installation is officially a Big Event, and tweakin
the ini is a one-time operation per upgrade, so I believe some people
may be overestimating the pain it will cause.

There won't be any bug reports, if the ini parser detects improper
backslash usage (i.e., using a backslash with a following character
which doesn't form correct escape combination) and stops with
informative and descriptive message explaining the parser changes. Then
anyone who can read, will figure it out, and fix it.

This is not really necessary change. We could only allow escaping only for "
which is propably the only thing you might need in certain cases.

I can't remember now what the bug was that this actually fixed (!) but there was
one related to putting double quotes in ini values..

--Jani

There won't be any bug reports, if the ini parser detects improper backslash
usage (i.e., using a backslash with a following character which doesn't form
correct escape combination) and stops with informative and descriptive
message explaining the parser changes. Then anyone who can read, will
figure it out, and fix it.

Did you actually read what Johannes said?
"c:\Documents\tony" is perfectly legal string in both cases.
But in 5.3 it means "c:\Documents<tab>ony" and this is THE problem.
No error messages, no explanation whatsoever.

--
Wbr,
Antony Dovgal

hi,

Hi!

I don't think something allowed and documented can be just remove without
considering the consequences. It worked and did what it was intended to do,
why we need to drop it?
If people actually have configs that would stop working in 5.3 and it
wouldn't be good for anybody. In windows, because of spaces, a lot of people
would have include_path with quotes, and also most of them use , because
they just copy-paste these paths.
I'm also not sure how you'd have these people fix it. Say you have path
like: "c:\Documents and Settings\John Doe\My Documents\PHP Projects\Cool
Site Project\include" - you'd have everybody to manually scan each such
string and replace \ with /? Or just drop final ?

Or simply remove the double quotes.

include_path=C:\Users\pierre\Documents\Digsby Logs

works just fine.

I think we are making a big problem of a non issue. This issue is easy
to catch and fix. If a problem occurs in an app, it will be visible
very quickly, like for missing extensions or wrong include_path.

Cheers,

Pierre
http://blog.thepimp.net | http://www.libgd.org

Hi!

Attached is the patch that does the following for php.ini parser:

1. {LETTER} is literal inside "" now, whatever the letter is
2. " is " inside "", with exception of:
3. foo="bar"{NEWLINE} is parsed as bar\ - i.e. \ is allowed before
   closing " but only if the line ends there (otherwise it's impossible to
   know what the intent was).

That should make Windows paths work just fine (unless you have some
really weird ones that I didn't think about) with double quotes.

Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com

Hannes Magnusson wrote:

Johannes Schlüter kirjoitti:

Hi,

now that we have a publicly available alpha on Windows we get the first
feedback from Windows users. Within quite short time we got two bug
reports (#45992 #45994) about a change in the ini-parsing:

With <=5.2 we allow

include_path = "c:\foo\bar"
That SHOULD cause a syntax error of course. If it doesn't,
it's the bug to fix. Allowing " was a bug that was fixed to begin with.
If that causes a syntax error, it's quite easy to catch and fix by people
upgrading to 5.3..

php.ini-recommended, 5.3:
;;;;;;;;;;;;;;;;;;;;;;;;;
; Paths and Directories ;
;;;;;;;;;;;;;;;;;;;;;;;;;

; UNIX: "/path1:/path2"
;include_path = ".:/php/includes"
;
; Windows: "\path1;\path2"
;include_path = ".;c:\php\includes"

As you can see, Windows users are explicitly told to use foo\bar
inside double quotes.
This has been like this for years and is also documented in the manual.

It looks to me that the old behavior was very intentional, logical or not.

Ini files are not documentation. And as / works also for winblows, that
should be used JUST for the sake of consistency anyway..

Anyway, the fix is quite easy: Only allow " and no other escapes. And
doing "foo" should cause a syntax error.

--Jani