phpwiki-talk

[Phpwiki-talk] Bug fix for removepage.php

[Oliver B. <ob...@de...>]

Hello All,

on Wednesday I posted this, maybe with an improper subject. Would nice to 
have some feedback.

Using PHPWiki 1.3.3, "Remove the page now" says "Someone has edited the 
page!..." or deletes HomePage if the version number of HomePage is 
accidentally identical to the version of the page to be deleted.

Cause: removepage.php looks for the actual page name by using
$request->getArg, therefore in HTTP_POST_VARS when hitting the "Remove the 
page now" button.

But HTTP_POST_VARS does not contain the page name (from the URI), at least 
on my system. So the HomePage is referenced.

To fix it, I inserted another hidden field in the remove form:

HTML::input(array('type' => 'hidden',
                  'name' => 'pagename',
                  'value' => $page->getName())),

It works, but I don't know whether $page->getName() is the best 
approach since I have little knowledge about the interplay of all the 
stuff.

Another possibility could be to merge HTTP_POST_VARS and HTTP_GET_VARS in 
Request(), any comment on this?

Could this relate to the search functions not working on (some?) 
systems whith USE_PATH_INFO=true? There was also missing the name of 
the search page...

Oliver

-- 
Oliver Betz, Muenchen

Re: [Phpwiki-talk] Bug fix for removepage.php

[Jeff D. <da...@da...>]

Thanks for the patch (and the repeated kicks), Oliver.

> Cause: removepage.php looks for the actual page name by using
> $request->getArg, therefore in HTTP_POST_VARS when hitting the "Remove
> the page now" button.

Yes, but not quite.  $Request is a WikiRequest, and the constructor
for WikiRequest fills in 'pagename' from some other sources if it's
not in HTTP_POST_VARS.  (If USE_PATH_INFO is true, 'pagename' gets
filled in from PATH_INFO --- the bug you're reporting only shows
up when USE_PATH_INFO is off...)

> Another possibility could be to merge HTTP_POST_VARS and HTTP_GET_VARS
> in Request(), any comment on this?

In general, I think that's a bad idea.  For security reasons, one should
ignore GET args on POST requests, I think.

In this case (deducing pagename), though, I think that's the best way
to fix the problem...

There's an asymmetry between USE_PATH_INFO and !USE_PATH_INFO.
When making a POST request with USE_PATH_INFO, the pagename
can (and should, I think) be communicated through the request
URL (via PATH_INFO).  When making a POST request without
USE_PATH_INFO, this can't be done --- if we want to allow the
pagename to be communicated in the URL, GET args is the only
way...  (This only applies to pagename --- other query args,
of course, cannot be communicated through PATH_INFO...)

So (shortly) I'm going to fix WikiRequest::_deducePagename() 
(in main.php) so that it'll be able to figure out pagename from
HTTP_GET_VARS...

> Could this relate to the search functions not working on (some?) 
> systems whith USE_PATH_INFO=true? There was also missing the name of 
> the search page...

It might be related to other problems with USE_PATH_INFO=false, but I
doubt it has to do with USE_PATH_INFO=true problems.  Of course, I've been
wrong before...

Jeff

Re: [Phpwiki-talk] Bug fix for removepage.php

[Oliver B. <ob...@de...>]

Jeff Dairiki wrote: 

[$request->getArg looks in HTTP_POST_VARS] 

> Yes, but not quite.  $Request is a WikiRequest, and the constructor
> for WikiRequest fills in 'pagename' from some other sources if it's

I see. 

[merge HTTP_POST_VARS and HTTP_GET_VARS] 

> In general, I think that's a bad idea.  For security reasons, one should

Ack. I only thought about 'pagename'. 

[...] 

> So (shortly) I'm going to fix WikiRequest::_deducePagename() 
> (in main.php) so that it'll be able to figure out pagename from
> HTTP_GET_VARS...

This works here: 

 if (isset($_GET['pagename']))     return $_GET['pagename']; 

Or $GLOBALS['HTTP_GET_VARS']['pagename'] for "old" PHP. 

but I have no clue whether this is good style. 


I assume that there is a reason for the nesting and against combining it 
in request(). 


[Could this relate to the search functions not working...] 


> It might be related to other problems with USE_PATH_INFO=false, but I
> doubt it has to do with USE_PATH_INFO=true problems.

I see, the problem is really the missing argument from the "action" of a 
"get" form. I checked also Opera 6.05 and Mozilla 1.1 with a simple form, 
but didn't find the argument passed to a PHP variable, so I wouldn't call 
it InternetExplorerGetMethodBug. So I use the fixes with hidden input 
fields. 


Oliver 
-- 
Oliver Betz, Muenchen

Re: [Phpwiki-talk] Bug fix for removepage.php

[Jeff D. <da...@da...>]

On Sun, 20 Oct 2002 21:46:26 +0200
"Oliver Betz" <ob...@de...> wrote:

> This works here: 
> 
>  if (isset($_GET['pagename']))     return $_GET['pagename']; 
> 
> Or $GLOBALS['HTTP_GET_VARS']['pagename'] for "old" PHP. 
> 
> but I have no clue whether this is good style. 

That's, essentially, what I did.  (Using $HTTP_GET_VARS rather
than $_GET, since I think we all decided a couple of months ago
that that's the way to go for now...)  (I sure wish those PHP
guys would stop changing basic things like that...)

> I assume that there is a reason for the nesting and against combining it
> 
> in request(). 

You mean WikiRequest vs. Request?  They could be combined with no
functional
loss --- I left them separate for both evolutinary and organizational
reason.
Request is completely non-wiki-specific.  It just deals with CGI request
things: gettings query args, setting cookies, etc...
WikiRequest deals with all the specifics of handling a request
to view/edit a PhpWiki page.  In retrospect WikiRequest should probably
_use_ Request rather than _being_ a Request.

(So anything that treats 'pagename' differently than any other
query arg should be in WikiRequest, not Request....)


> I see, the problem is really the missing argument from the "action" of a
> 
> "get" form. I checked also Opera 6.05 and Mozilla 1.1 with a simple
> form, but didn't find the argument passed to a PHP variable, so I
> wouldn't call it InternetExplorerGetMethodBug. So I use the fixes with
> hidden input fields. 

Is this still a problem in the current CVS code?  If so, can you provide
me a few details?

Re: [Phpwiki-talk] Bug fix for removepage.php

[Oliver B. <ob...@de...>]

Jeff Dairiki wrote:

[...]

> > I assume that there is a reason for the nesting and against combining it
> > in request(). 
> 
> You mean WikiRequest vs. Request?  They could be combined with no

Yes, especially things like _deduceSomething() in the constructor. Thanks 
for the explanation, now the reason is obvious.

[missing argument from the "action" of a "get" form]
 
> Is this still a problem in the current CVS code?

No - I don't think so because I added the same hidden fields to the 
template(s) and it works now. The snapshot doesn't run here and I still 
had no time to investigate it further.

I still wonder why the argument is passed with a POST request but not with 
a GET but stop bother about.

After all, with these patches the 1.3.3 version runs very well with 
USE_PATH_INFO set to "false" and I hope now to be able to convince some 
people to share knowledge. I'm afraid that's more difficult than fixing 
some minor problems in PHPWiki.

Thanks again,

Oliver
-- 
Oliver Betz, Muenchen