The action would not be set if no valid project was found in
path_info. Removing the return if the project was not specified fixes
the project_index and opml actions when using path_info.
Signed-off-by: Devin Doucette <redacted>
---
gitweb/gitweb.perl | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
@@ -552,8 +552,7 @@ sub evaluate_path_info {while($project&&!check_head_link("$projectroot/$project")){$project=~s,/*[^/]*$,,;}-returnunless$project;-$input_params{'project'}=$project;+$input_params{'project'}=$projectif$project;# do not change any parameters if an action is given using the query stringreturnif$input_params{'action'};
From: Jakub Narebski <hidden> Date: 2016-06-15 22:45:51
On Sun, 28 Dec 2008, Devin Doucette wrote:
The action would not be set if no valid project was found in
path_info. Removing the return if the project was not specified fixes
the project_index and opml actions when using path_info.
Thanks for catching this.
Truth to be told we parse action parameter in path_info only since
d8c2882 (gitweb: parse project/action/hash_base:filename PATH_INFO)
by Giuseppe Bilotta (CC-ed; I think he is correct person to give
Ack for this patch). Earlier only "default" actions could be expressed
using only path_info, and project-less 'opml' and 'project_index'
actions are not default actions for projectless URL, so there was no
such problem then.
@@ -552,8 +552,7 @@ sub evaluate_path_info {while($project&&!check_head_link("$projectroot/$project")){$project=~s,/*[^/]*$,,;}-returnunless$project;-$input_params{'project'}=$project;+$input_params{'project'}=$projectif$project;# do not change any parameters if an action is given using the query stringreturnif$input_params{'action'};
From: Giuseppe Bilotta <hidden> Date: 2016-06-15 22:45:51
On Fri, Jan 2, 2009 at 12:58 AM, Jakub Narebski [off-list ref] wrote:
Truth to be told we parse action parameter in path_info only since
d8c2882 (gitweb: parse project/action/hash_base:filename PATH_INFO)
by Giuseppe Bilotta (CC-ed; I think he is correct person to give
Ack for this patch). Earlier only "default" actions could be expressed
using only path_info, and project-less 'opml' and 'project_index'
actions are not default actions for projectless URL, so there was no
such problem then.
Actually, the early bailout was sort of intentional. The problem is
the ambiguity: does git.example.com/opml refer to the opml project, or
does it refer to the opml action?
HOWEVER, href() *does* create the opml action as git.example.com/opml,
so gitweb is currently broken in the sense that ti doesn't correctly
parse its own pathinfo output. So the question is: shall we go with
this patch, preventing pathinfo from working for projects named like a
no-project gitweb action, or should we fix href() to not generate
pathinfo unless project is defined?
Note that if this patch is accepted, we probably need an appropriate
patch in href() anyway to use query params for projects named like
no-project actions.
--
Giuseppe "Oblomov" Bilotta
On Thu, Jan 1, 2009 at 5:46 PM, Giuseppe Bilotta
[off-list ref] wrote:
Actually, the early bailout was sort of intentional. The problem is
the ambiguity: does git.example.com/opml refer to the opml project, or
does it refer to the opml action?
Good point. Though my patch does not break any existing functionality,
it does not fix the case where a project matches the action.
HOWEVER, href() *does* create the opml action as git.example.com/opml,
so gitweb is currently broken in the sense that ti doesn't correctly
parse its own pathinfo output. So the question is: shall we go with
this patch, preventing pathinfo from working for projects named like a
no-project gitweb action, or should we fix href() to not generate
pathinfo unless project is defined?
A variation of the latter approach is to modify href() to use pathinfo
if there is a project or there is no project matching the name of the
action. The only downside to this approach is that a URI that refers to
an action when first generated could resolve to a project in the future,
if a project of the same name were added.
--
Devin Doucette
From: Giuseppe Bilotta <hidden> Date: 2016-06-15 22:45:51
On Fri, Jan 2, 2009 at 3:21 AM, Devin Doucette [off-list ref] wrote:
On Thu, Jan 1, 2009 at 5:46 PM, Giuseppe Bilotta
[off-list ref] wrote:
quoted
Actually, the early bailout was sort of intentional. The problem is
the ambiguity: does git.example.com/opml refer to the opml project, or
does it refer to the opml action?
Good point. Though my patch does not break any existing functionality,
it does not fix the case where a project matches the action.
quoted
HOWEVER, href() *does* create the opml action as git.example.com/opml,
so gitweb is currently broken in the sense that ti doesn't correctly
parse its own pathinfo output. So the question is: shall we go with
this patch, preventing pathinfo from working for projects named like a
no-project gitweb action, or should we fix href() to not generate
pathinfo unless project is defined?
A variation of the latter approach is to modify href() to use pathinfo
if there is a project or there is no project matching the name of the
action. The only downside to this approach is that a URI that refers to
an action when first generated could resolve to a project in the future,
if a project of the same name were added.
And don't forget the case of a new action being introduced which is
named like an existing project.
I suspect we'd be much better off by not using pathinfo at all for
global actions. Other possible solutions would be to have something
like an ADDITIONAL prefix to the path (like project/projectname vs
action/actionname, so we would have git.example.com/action/opml or
git.example.com/project/example-project) but this is WAY too verbose.
At most, we could add a suffix to global actions to reduce the chance
of name conflict, such as using
git.example.com/opml.xml
or
git.example.com/project_index.txt
but I don't like these solution either: I'd rather have these be
provided as filenames in the response header.
Incoming patch to not make use of path_info without project.
--
Giuseppe "Oblomov" Bilotta