* Re: Strange behaviour of git diff branch1 ... branch2
From: Junio C Hamano @ 2012-11-12 17:46 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git, Aaron Schrab
In-Reply-To: <CACsJy8BkKRxp9AQW0M0vDSL8Mb16VGSrZ1OnQpwCrERWKVLSyA@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> On Sat, Oct 27, 2012 at 7:33 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
>>>
>>>Notice the --cc in the first line, which is combined diff. Usually
>>>combined-diff is between two points and one parent. Though somehow git
>>>passes 4 parents down combined-diff.c:show_combined_header, as you can
>>>see in the "index" line. I think we should fix rev parsing code as it
>>>does not make sense to pass 4 identical parents this way.
>>
>> The two heads home from HEAD...HEAD the user has on the command line.
>>
>> The user is getting exactly what she asked; there is nothing to fix.
>
> Is there any use case where HEAD...HEAD (or "..." alone) is actually useful?
That is an invalid question, implying a proposal that will introduce
more confusion to the users while hurting the code.
There is no use case where "cat A >>A" is actually useful, either,
but it does not mean "cat A >>A" should be special cased to do
something different from what it happens to do in line with the case
where "cat A >>B" does. I think "diff ..." or "diff HEAD...HEAD"
are the same as that example.
^ permalink raw reply
* Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
From: Junio C Hamano @ 2012-11-12 17:47 UTC (permalink / raw)
To: Michael Haggerty, David Aguilar
Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git,
Lars R. Damerow, Jeff King, Marc Jordan
In-Reply-To: <508E0FAC.5050600@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> The log message of the original commit (0454dd93bf) described the
> following scenario: a /home partition under which user home directories
> are automounted, and setting GIT_CEILING_DIRECTORIES=/home to avoid
> hitting /home/.git, /home/.git/objects, and /home/objects (which would
> attempt to automount those directories). I believe that this scenario
> would not be slowed down by my patches.
>
> How do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a
> slowdown?
Yeah, I was also wondering about that.
David?
>> Is there another way to accomplish this without the performance hit?
>> Maybe something that can be solved with configuration?
>
> Without doing the symlink expansion there is no way for git to detect
> that GIT_CEILING_DIRECTORIES contains symlinks and is therefore
> ineffective. So the user has no warning about the misconfiguration
> (except that git runs slowly).
>
> On 10/29/2012 02:42 AM, Junio C Hamano wrote:
>> Perhaps not canonicalize elements on the CEILING list ourselves? If
>> we make it a user error to put symlinked alias in the variable, and
>> document it clearly, wouldn't it suffice?
>
> There may be no other choice. (That, and fix the test suite in another
> way to tolerate a $PWD that involves symlinks.)
^ permalink raw reply
* Re: [PATCHv3 3/4] git-status: show short sequencer state
From: Phil Hord @ 2012-11-12 18:14 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, phil.hord, Jeff King, konglu, Matthieu Moy, Kong Lucien,
Duperray Valentin, Jonas Franck, Nguy Thomas
In-Reply-To: <7vip9aiuk8.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Phil Hord <hordp@cisco.com> writes:
>
>> State token strings which may be emitted and their meanings:
>> merge a merge is in progress
>> am an am is in progress
>> am-is-empty the am patch is empty
>> rebase a rebase is in progress
>> rebase-interactive an interactive rebase is in progress
>> cherry-pick a cherry-pick is in progress
>> bisect a bisect is in progress
>> conflicted there are unresolved conflicts
>> commit-pending a commit operation is waiting to be completed
>> splitting interactive rebase, commit is being split
>>
>> I also considered adding these tokens, but I decided it was not
>> appropriate since these changes are not sequencer-related. But
>> it is possible I am being too short-sighted or have chosen the
>> switch name poorly.
>> changed-index Changes exist in the index
>> changed-files Changes exist in the working directory
>> untracked New files exist in the working directory
> I tend to agree; unlike all the normal output from "status -s" that
> are per-file, the above are the overall states of the working tree.
>
> It is just that most of the "overall states" look as if they are
> dominated by "sequencer states", but that is only because you chose
> to call states related to things like "am" and "bisect" that are not
> sequencer states as such.
>
> It probably should be called the tree state, working tree state, or
> somesuch.
I think you are agreeing that I chose the switch name poorly, right?
Do you think '--tree-state' is an acceptable switch or do you have other
suggestions?
Phil
^ permalink raw reply
* Re: splitting off shell test framework
From: Drew Northup @ 2012-11-12 18:18 UTC (permalink / raw)
To: Adam Spiers; +Cc: Git Mailing List, felipe.contreras, Jason J Pyeron CTR (US)
In-Reply-To: <CAOkDyE8KxFvM4CJhC4U=Jb95D6HQ-4qQBtKAgBMyHH15UOhvqg@mail.gmail.com>
On Mon, Nov 12, 2012 at 11:37 AM, Adam Spiers <git@adamspiers.org> wrote:
> Hi all,
>
> I've been pretty impressed with git's test framework, and I'm not
> aware of many other (decent) shell-based test frameworks out there.
> (One that springs to mind is the one used by rvm, but last time I
> looked - admittedly a while ago now - it had limitations).
>
> Recently a situation arose where I craved the ability to test
> something via shell. I did a quick proof of concept and successfully
> extracted out the non-git-specific bits of git's test framework to be
> used to test something entirely unrelated to git:
>
> https://github.com/aspiers/shell-env/tree/master/t
>
> As it turned out to be fairly easy, I was wondering if there would be
> any interest in doing this more formally, i.e. splitting off the
> framework so that it could be used and improved outside the scope of
> git development? Of course this would pose the question how git would
> consume this new project without any risk of destabilisation. I'm
> guessing that simply using a git submodule would solve the problem,
> but ICBW ...
>
> Just an idea. Interesting, or terrible? :)
Done at least once already:
http://comments.gmane.org/gmane.comp.version-control.git/201591
--
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59
^ permalink raw reply
* Re: [PATCHv3 1/4] Refactor print_state into get_state
From: Ramkumar Ramachandra @ 2012-11-12 18:29 UTC (permalink / raw)
To: Phil Hord
Cc: git, phil.hord, Jeff King, Junio C Hamano, konglu, Matthieu Moy,
Kong Lucien, Duperray Valentin, Jonas Franck, Nguy Thomas
In-Reply-To: <1352487385-5929-2-git-send-email-hordp@cisco.com>
Phil Hord wrote:
> Recently git-status learned to display the state of the git
> sequencer in long form to help the user remember an interrupted
> command. This information is useful to other callers who do
> not want it printed in the same way.
Nit: Please mention that "Recently" refers to 83c750ac to save future
readers the trouble of running blame.
Thanks.
Ram
^ permalink raw reply
* Re: [BUG] gitweb: XSS vulnerability of RSS feed
From: Drew Northup @ 2012-11-12 18:55 UTC (permalink / raw)
To: glpk xypron; +Cc: git, jnareb, Junio C Hamano
In-Reply-To: <20121111232820.284510@gmx.net>
On Sun, Nov 11, 2012 at 6:28 PM, glpk xypron <xypron.glpk@gmx.de> wrote:
> Gitweb can be used to generate an RSS feed.
>
> Arbitrary tags can be inserted into the XML document describing
> the RSS feed by careful construction of the URL.
>
> Example
> http://server/?p=project.git&a=rss&f=</title><script>alert(document.cookie)</script><title>
>
> The generated XML contains
> <script>alert(document.cookie)</script>
>
> Depending on the system used to render the XML this might lead
> to the execution of javascript in the security context of the
> gitweb server pages.
>
> Please, escape all URL parameters.
>
> Version tested:
> gitweb v.1.8.0.dirty with git 1.7.2.5
>
> Best regards
>> Heinrich Schuchardt
Something like this may be useful to defuse the "file" parameter, but
I presume a more definitive fix is in order...
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 10ed9e5..af93e65 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1447,6 +1447,10 @@ sub validate_pathname {
if ($input =~ m!\0!) {
return undef;
}
+ # No XSS <script></script> inclusions
+ if ($input =~ m!(<script>)(.*)(</script>)!){
+ return undef;
+ }
return $input;
}
(I am not a perl god, so this was the lowest hanging fruit.)
If desired I'll fashion this up into a proper patch.
--
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59
^ permalink raw reply related
* Re: [Patch 1/1] Wire html for all files in ./technical and ./howto in Makefile
From: Jeff King @ 2012-11-12 19:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Thomas Ackermann, git
In-Reply-To: <7vpq3iiukk.fsf@alter.siamese.dyndns.org>
On Mon, Nov 12, 2012 at 09:45:31AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > On Mon, Oct 29, 2012 at 07:33:47PM +0100, Thomas Ackermann wrote:
> >
> >> This patch addresses Junios comment in WC:
> >> "Misapplication of a patch fixed; the ones near the tip needs to
> >> update the links to point at the html files, though."
> >>
> >> See older mail in this thread:
> >> [...]
> >> That means that for the patch [6/8], which adds content-type to the
> >> text files, to be complete, it needs to update Makefile to produce
> >> html files from them.
> >> [...]
> >> So IMHO no open issues with this patch.
> >
> > OK, that explains the situation. Thanks, I'll merge it to master in the
> > next iteration.
>
> What happened to this topic?
I mistyped in the above response; it should have been "next" not
"master".
But then I forgot to merge it to next in the last iteration; it is
ta/doc-cleanup in pu, and is marked in "What's Cooking" to be merged to
next.
-Peff
^ permalink raw reply
* Re: [regression] Newer gits cannot clone any remote repos
From: Douglas Mencken @ 2012-11-12 19:35 UTC (permalink / raw)
To: Kevin; +Cc: git
In-Reply-To: <CAO54GHC9ibTwxqLnk1vGSo9R011HE05JOL1mgpTGzeWX7cCcwQ@mail.gmail.com>
> On Mon, Nov 12, 2012 at 12:12 PM, Kevin <ikke@ikke.info> wrote:
> > Maybe handy to say that you're on a Powerpc platform.
Oh, and yes, I'm on 2 x 2-core ("4-core") machine.
^ permalink raw reply
* Re: [PATCH 1/2] git-sh-setup: refactor ident-parsing functions
From: Jeff King @ 2012-11-12 19:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Ilya Basin, git
In-Reply-To: <7vpq3ik97i.fsf@alter.siamese.dyndns.org>
On Mon, Nov 12, 2012 at 09:44:01AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > The only ident-parsing function we currently provide is
> > get_author_ident_from_commit. This is not very
> > flexible for two reasons:
> >
> > 1. It takes a commit as an argument, and can't read from
> > commit headers saved on disk.
> >
> > 2. It will only parse authors, not committers.
> >
> > This patch provides a more flexible interface which will
> > parse multiple idents from a commit provide on stdin. We can
> > easily use it as a building block for the current function
> > to retain compatibility.
> >
> > Signed-off-by: Jeff King <peff@peff.net>
> > ---
> > Since we are counting processes in this series, I should note that this
> > actually adds a subshell invocation for each call, since it went from:
> >
> > script='...'
> > sed $script
> >
> > to:
> >
> > sed "$(make_script)"
> >
> > For filter-branch, which is really the only high-performance caller we
> > have, this is negated by the fact that it will do author and committer
> > at the same time, saving us an extra subshell (in addition to an extra
> > sed invocation).
>
> Given that pick-ident-script is a const function, a caller that
> repeatedly call is could call it once and use it in a variable, no?
The problem is that it is a helper called from parse_ident_from_commit.
And that function just passes along its arguments, so it does not know
that it is being called repeatedly with the same arguments. So you'd
have to either change the interface or memoize internally.
I don't think memoization is a good option for two reasons:
1. Storing the arguments to compare to later is complex. You don't
want to just store "$*" from the last run and see if we got the
same arguments. You'd have to quote your delimiter (e.g., you would
not want to confuse ("foo", "bar") with ("foo bar"). Though in this
instance, we know that our args do not have spaces, so we could get
away with that.
2. If you are in a subshell or even a while loop, your memoized
variable will not be retained.
So unless somebody has some clever scheme for memoizing shell functions
without any process overhead, it is probably not worth it.
Changing the interface for get_author_ident_from_commit would be a pain,
but if we just wanted to help filter-branch, we could do something like
this:
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 5314249..7a693ba 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -74,7 +74,7 @@ finish_ident() {
}
set_ident () {
- parse_ident_from_commit author AUTHOR committer COMMITTER
+ parse_ident_from_commit_via_script "$ident_script"
finish_ident AUTHOR
finish_ident COMMITTER
}
@@ -93,6 +93,7 @@ if [ "$(is_bare_repository)" = false ]; then
require_clean_work_tree 'rewrite branches'
fi
+ident_script=$(pick_ident_script author AUTHOR committer COMMITTER)
tempdir=.git-rewrite
filter_env=
filter_tree=
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 22f0aed..1e20e17 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -225,10 +225,17 @@ pick_ident_script () {
echo '/^$/q'
}
+# Feed a pick_ident_script return value to sed. Use this instead of
+# parse_ident_from_commit below if you are going to be parsing commits in a
+# tight loop and want to save a process.
+parse_ident_from_commit_via_script() {
+ LANG=C LC_ALL=C sed -ne "$1"
+}
+
# Create a pick-script as above and feed it to sed. Stdout is suitable for
# feeding to eval.
parse_ident_from_commit () {
- LANG=C LC_ALL=C sed -ne "$(pick_ident_script "$@")"
+ parse_ident_from_commit_via_script "$(pick_ident_script "$@")"
}
# Parse the author from a commit given as an argument. Stdout is suitable for
^ permalink raw reply related
* Re: [PATCH 2/5] launch_editor: ignore SIGINT while the editor has control
From: Jeff King @ 2012-11-12 19:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Paul Fox, Kalle Olavi Niemitalo, git
In-Reply-To: <7v4nkuk966.fsf@alter.siamese.dyndns.org>
On Mon, Nov 12, 2012 at 09:44:49AM -0800, Junio C Hamano wrote:
> How did this message happen?
>
> Subject: [PATCH 2/5] launch_editor: ignore SIGINT while the editor has control
> To: Kalle Olavi Niemitalo <kon@iki.fi>
> Cc: Paul Fox <pgf@foxharp.boston.ma.us>, git@vger.kernel.org
> Date: Sun, 11 Nov 2012 11:55:11 -0500
> Message-ID: <20121111165510.GB19850@sigill.intra.peff.net>
> References: <20121111163100.GB13188@sigill.intra.peff.net>
>
> The user's editor likely catches SIGINT (ctrl-C). but if
> the user spawns a command from the editor and uses ctrl-C to
> kill that command, the SIGINT will likely also kill git
> itself (depending on the editor, this can leave the terminal
> in an unusable state).
>
> Signed-off-by: Paul Fox <pgf@foxharp.boston.ma.us>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>
> Judging from S-o-b, message-id and EHLO, I think this was sent by
> Peff, but came without Sender: or anything.
>
> Just being curious.
I screwed up when sending out the series and did not properly move
Paul's "From" address from the email header down to the body. It is not
a git or send-email screw-up; I load format-patch output directly into
mutt as a template. Since I do not often send out other people's
patches, I never bothered to write a script to migrate the "from" into
the body automatically.
-Peff
^ permalink raw reply
* Fwd: [PATCH] Add tcsh-completion support to contrib by using git-completion.bash
From: Marc Khouzam @ 2012-11-12 20:07 UTC (permalink / raw)
To: git
In-Reply-To: <CAFj1UpE6OtJEojaED1_DZJD0kU=nVsFE_w8xa0oJE-6auCU2rw@mail.gmail.com>
Hi,
this patch allows tcsh-users to get the benefits of the awesome
git-completion.bash script. It could also help other shells do the same.
==
The current tcsh-completion support for Git, as can be found on the
internet, takes the approach of defining the possible completions
explicitly. This has the obvious draw-back to require constant
updating as the Git code base evolves.
The approach taken by this commit is to to re-use the advanced bash
completion script and use its result for tcsh completion. This is
achieved by executing (versus sourcing) the bash script and
outputting the completion result for tcsh consumption.
Three solutions were looked at to implement this approach with (A)
being retained:
A) Modifications:
git-completion.bash and new git-completion.tcsh
Modify the existing git-completion.bash script to support
being sourced using bash (as now), but also executed using bash.
When being executed, the script will output the result of the
computed completion to be re-used elsewhere (e.g., in tcsh).
Pros:
1- allows the git-completion.bash script to easily be re-used
2- tcsh support is mostly isolated in git-completion.tcsh
Cons (for tcsh users only):
1- requires the user to copy both git-completion.tcsh and
git-completion.bash to ${HOME}
2- requires bash script to have a fixed name and location:
${HOME}/.git-completion.bash
B) Modifications:
git-completion.bash
Modify the existing git-completion.bash script to support
being sourced using bash (as now), but also executed using bash,
and sourced using tcsh.
Pros:
1- only requires the user to deal with a single file
2- maintenance more obvious for tcsh since it is entirely part
of the same git-completion.bash script.
Cons:
1- tcsh support could affect bash support as they share the
same script
2- small tcsh section must use syntax suitable for both tcsh
and bash and must be at the beginning of the script
3- requires the user to explicitly make the script executable
when using tcsh (for tcsh users only)
4- requires script to have a fixed name and location:
${HOME}/.git-completion.sh (for tcsh users only)
C) Modifications:
New git-completion.tcsh
Provide a short tcsh script that converts git-completion.bash
into an executable script suitable to be used by tcsh.
Pros:
1- tcsh support is entirely isolated in git-completion.tcsh
2- new tcsh script can be as complex as needed
Cons (for tcsh users only):
1- requires the user to copy both git-completion.tcsh and
git-completion.bash to ${HOME}
2- requires bash script to have a fixed name and location:
${HOME}/.git-completion.bash
3- sourcing the new script will generate a third script
Approach (A) was selected to keep the tcsh completion support well
isolated without introducing excessive complexity.
Signed-off-by: Marc Khouzam <marc.khouzam@gmail.com>
==
With the changes applied, tcsh users should:
# 1) Copy both this file and the bash completion script to your
${HOME} directory
# using the names ${HOME}/.git-completion.tcsh and
${HOME}/.git-completion.bash.
# 2) Add the following line to your .tcshrc/.cshrc:
# source ${HOME}/.git-completion.tcsh
The code can be found on GitHub.
Option (A):
https://github.com/marckhouzam/git/commit/86d3a8e740ae85b4b4462c997a0fd969b1b2d24c
Option (B):
https://github.com/marckhouzam/git/commit/e64606541682eaf66c0a56aceff279ca6e1d06cd
Option (C):
https://github.com/marckhouzam/git/commit/59792455f1e6a98d3ffeb828f4cff1ded0e4ed37
Thanks
Marc
---
contrib/completion/git-completion.bash | 53 +++++++++++++++++++++++++++++++-
contrib/completion/git-completion.tcsh | 34 ++++++++++++++++++++
2 files changed, 86 insertions(+), 1 deletions(-)
create mode 100755 contrib/completion/git-completion.tcsh
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index be800e0..6d4b57a 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1,4 +1,6 @@
-#!bash
+#!/bin/bash
+# The above line is important as this script can be executed when used
+# with another shell such as tcsh
#
# bash/zsh completion support for core Git.
#
@@ -2481,3 +2483,52 @@ __git_complete gitk __gitk_main
if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then
__git_complete git.exe __git_main
fi
+
+# Method that will output the result of the completion done by
+# the bash completion script, so that it can be re-used in another
+# context than the bash complete command.
+# It accepts 1 to 2 arguments:
+# 1: The command-line to complete
+# 2: The index of the word within argument #1 in which the cursor is
+# located (optional). If parameter 2 is not provided, it will be
+# determined as best possible using parameter 1.
+_git_complete_with_output ()
+{
+ # Set COMP_WORDS to the command-line as bash would.
+ COMP_WORDS=($1)
+
+ # Set COMP_CWORD to the cursor location as bash would.
+ if [ -n "$2" ]; then
+ COMP_CWORD=$2
+ else
+ # Assume the cursor is at the end of parameter #1.
+ # We must check for a space as the last character which will
+ # tell us that the previous word is complete and the cursor
+ # is on the next word.
+ if [ "${1: -1}" == " " ]; then
+ # The last character is a space, so our
location is at the end
+ # of the command-line array
+ COMP_CWORD=${#COMP_WORDS[@]}
+ else
+ # The last character is not a space, so our
location is on the
+ # last word of the command-line array, so we
must decrement the
+ # count by 1
+ COMP_CWORD=$((${#COMP_WORDS[@]}-1))
+ fi
+ fi
+
+ # Call _git() or _gitk() of the bash script, based on the first
+ # element of the command-line
+ _${COMP_WORDS[0]}
+
+ # Print the result that is stored in the bash variable ${COMPREPLY}
+ for i in ${COMPREPLY[@]}; do
+ echo "$i"
+ done
+}
+
+if [ -n "$1" ] ; then
+ # If there is an argument, we know the script is being executed
+ # so go ahead and run the _git_complete_with_output function
+ _git_complete_with_output "$1" "$2"
+fi
diff --git a/contrib/completion/git-completion.tcsh
b/contrib/completion/git-completion.tcsh
new file mode 100755
index 0000000..7b7baea
--- /dev/null
+++ b/contrib/completion/git-completion.tcsh
@@ -0,0 +1,34 @@
+#!tcsh
+#
+# tcsh completion support for core Git.
+#
+# Copyright (C) 2012 Marc Khouzam <marc.khouzam@gmail.com>
+# Distributed under the GNU General Public License, version 2.0.
+#
+# This script makes use of the git-completion.bash script to
+# determine the proper completion for git commands under tcsh.
+#
+# To use this completion script:
+#
+# 1) Copy both this file and the bash completion script to your
${HOME} directory
+# using the names ${HOME}/.git-completion.tcsh and
${HOME}/.git-completion.bash.
+# 2) Add the following line to your .tcshrc/.cshrc:
+# source ${HOME}/.git-completion.tcsh
+
+# One can change the below line to use a different location
+set __git_tcsh_completion_script = ${HOME}/.git-completion.bash
+
+# Check that the user put the script in the right place
+if ( ! -e ${__git_tcsh_completion_script} ) then
+ echo "ERROR in git-completion.tcsh script. Cannot find:
${__git_tcsh_completion_script}. Git completion will not work."
+ exit
+endif
+
+# Make the script executable if it is not
+if ( ! -x ${__git_tcsh_completion_script} ) then
+ chmod u+x ${__git_tcsh_completion_script}
+endif
+
+complete git 'p/*/`${__git_tcsh_completion_script} "${COMMAND_LINE}"
| sort | uniq`/'
+complete gitk 'p/*/`${__git_tcsh_completion_script} "${COMMAND_LINE}"
| sort | uniq`/'
+
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 1/2] git-sh-setup: refactor ident-parsing functions
From: Junio C Hamano @ 2012-11-12 20:08 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, Ilya Basin, git
In-Reply-To: <20121112194434.GB4623@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Changing the interface for get_author_ident_from_commit would be a pain,
> but if we just wanted to help filter-branch, we could do something like
> this:
Yes, that is the direction I was alluding to.
Callers of get_author_ident_from_commit can also do the same and
avoid rebuilding the same $pick_author_script over and over again,
or get_author_ident_from_commit can do so for its callers.
> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> index 5314249..7a693ba 100755
> --- a/git-filter-branch.sh
> +++ b/git-filter-branch.sh
> @@ -74,7 +74,7 @@ finish_ident() {
> }
>
> set_ident () {
> - parse_ident_from_commit author AUTHOR committer COMMITTER
> + parse_ident_from_commit_via_script "$ident_script"
> finish_ident AUTHOR
> finish_ident COMMITTER
> }
> @@ -93,6 +93,7 @@ if [ "$(is_bare_repository)" = false ]; then
> require_clean_work_tree 'rewrite branches'
> fi
>
> +ident_script=$(pick_ident_script author AUTHOR committer COMMITTER)
> tempdir=.git-rewrite
> filter_env=
> filter_tree=
> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index 22f0aed..1e20e17 100644
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -225,10 +225,17 @@ pick_ident_script () {
> echo '/^$/q'
> }
>
> +# Feed a pick_ident_script return value to sed. Use this instead of
> +# parse_ident_from_commit below if you are going to be parsing commits in a
> +# tight loop and want to save a process.
> +parse_ident_from_commit_via_script() {
> + LANG=C LC_ALL=C sed -ne "$1"
> +}
> +
> # Create a pick-script as above and feed it to sed. Stdout is suitable for
> # feeding to eval.
> parse_ident_from_commit () {
> - LANG=C LC_ALL=C sed -ne "$(pick_ident_script "$@")"
> + parse_ident_from_commit_via_script "$(pick_ident_script "$@")"
> }
>
> # Parse the author from a commit given as an argument. Stdout is suitable for
^ permalink raw reply
* Re: [PATCH 1/2] git-sh-setup: refactor ident-parsing functions
From: Jeff King @ 2012-11-12 20:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Ilya Basin, git
In-Reply-To: <7vzk2mh9dm.fsf@alter.siamese.dyndns.org>
On Mon, Nov 12, 2012 at 12:08:37PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Changing the interface for get_author_ident_from_commit would be a pain,
> > but if we just wanted to help filter-branch, we could do something like
> > this:
>
> Yes, that is the direction I was alluding to.
>
> Callers of get_author_ident_from_commit can also do the same and
> avoid rebuilding the same $pick_author_script over and over again,
> or get_author_ident_from_commit can do so for its callers.
I don't think get_author_ident_from_commit can do so on demand due to
the subshell issue I mentioned. So you'd have to generate the pick
script on inclusion of git-sh-setup, whether the includer wants to call
the function or not.
I wonder if we should simply generate these at build time and store them
in the script.
I'm also not sure that saving one process invocation is worth spending a
lot of thought cycles on. Maybe somebody on Windows could run a
filter-branch with the patch I just sent and measure whether it even
makes a difference.
-Peff
^ permalink raw reply
* Re: [Patch 1/1] Wire html for all files in ./technical and ./howto in Makefile
From: Junio C Hamano @ 2012-11-12 20:13 UTC (permalink / raw)
To: Jeff King; +Cc: Thomas Ackermann, git
In-Reply-To: <20121112191953.GA4623@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Nov 12, 2012 at 09:45:31AM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>> > On Mon, Oct 29, 2012 at 07:33:47PM +0100, Thomas Ackermann wrote:
>> >
>> >> This patch addresses Junios comment in WC:
>> >> "Misapplication of a patch fixed; the ones near the tip needs to
>> >> update the links to point at the html files, though."
>> >>
>> >> See older mail in this thread:
>> >> [...]
>> >> That means that for the patch [6/8], which adds content-type to the
>> >> text files, to be complete, it needs to update Makefile to produce
>> >> html files from them.
>> >> [...]
>> >> So IMHO no open issues with this patch.
>> >
>> > OK, that explains the situation. Thanks, I'll merge it to master in the
>> > next iteration.
>>
>> What happened to this topic?
>
> I mistyped in the above response; it should have been "next" not
> "master".
Ah, ok, I was just wondering why I didn't see it in your 'master'.
Thanks.
> But then I forgot to merge it to next in the last iteration; it is
> ta/doc-cleanup in pu, and is marked in "What's Cooking" to be merged to
> next.
>
> -Peff
^ permalink raw reply
* Re: [BUG] gitweb: XSS vulnerability of RSS feed
From: Jeff King @ 2012-11-12 20:24 UTC (permalink / raw)
To: Drew Northup; +Cc: glpk xypron, git, jnareb, Junio C Hamano
In-Reply-To: <CAM9Z-n=6xsC7yiKJ+NU-CxNPxEXWmJzvXLUocgZgWPQnuK6G4Q@mail.gmail.com>
On Mon, Nov 12, 2012 at 01:55:46PM -0500, Drew Northup wrote:
> On Sun, Nov 11, 2012 at 6:28 PM, glpk xypron <xypron.glpk@gmx.de> wrote:
> > Gitweb can be used to generate an RSS feed.
> >
> > Arbitrary tags can be inserted into the XML document describing
> > the RSS feed by careful construction of the URL.
> [...]
> Something like this may be useful to defuse the "file" parameter, but
> I presume a more definitive fix is in order...
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 10ed9e5..af93e65 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1447,6 +1447,10 @@ sub validate_pathname {
> if ($input =~ m!\0!) {
> return undef;
> }
> + # No XSS <script></script> inclusions
> + if ($input =~ m!(<script>)(.*)(</script>)!){
> + return undef;
> + }
> return $input;
> }
This is the wrong fix for a few reasons:
1. It is on the input-validation side, whereas the real problem is on
the output-quoting side. Your patch means I could not access a file
called "<script>foo</script>". What we really want is to have the
unquoted name internally, but then make sure we quote it when
outputting as part of an HTML (or XML) file.
2. Script tags are only part of the problem. They are what make it
obviously a security vulnerability, but it is equally incorrect for
us to show the filename "<b>foo</b>" as bold. I would also not be
surprised if there are other cross-site attacks one can do without
using <script>.
3. Your filter is too simplistic. At the very least, it would not
filter out "<SCRIPT>". I am not up to date on all of the
sneaking-around-HTML-filters attacks that are available these days,
but I wonder if one could also get around it using XML entities or
similar.
I think the right answer is going to be a well-placed call to esc_html.
This already happens automatically when we go through the CGI
element-building functions, but obviously we failed to make the call
when building the output manually. This is a great reason why template
languages which default to safe expansion should always be used.
Unfortunately, gitweb is living in 1995 in terms of web frameworks.
-Peff
^ permalink raw reply
* Re: [BUG] gitweb: XSS vulnerability of RSS feed
From: Jeff King @ 2012-11-12 20:27 UTC (permalink / raw)
To: Drew Northup; +Cc: glpk xypron, git, jnareb, Junio C Hamano
In-Reply-To: <20121112202413.GD4623@sigill.intra.peff.net>
On Mon, Nov 12, 2012 at 03:24:13PM -0500, Jeff King wrote:
> I think the right answer is going to be a well-placed call to esc_html.
I'm guessing the right answer is this:
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 10ed9e5..a51a8ba 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -8055,6 +8055,7 @@ sub git_feed {
$feed_type = 'history';
}
$title .= " $feed_type";
+ $title = esc_html($title);
my $descr = git_get_project_description($project);
if (defined $descr) {
$descr = esc_html($descr);
but I did not test it (and I am not that familiar with gitweb, so it is
a slight guess from spending 5 minutes grepping and reading).
-Peff
^ permalink raw reply related
* Re: [PATCH 1/4] remote-hg: add missing config for basic tests
From: Jeff King @ 2012-11-12 20:32 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Ramkumar Ramachandra
In-Reply-To: <1352742068-15346-2-git-send-email-felipe.contreras@gmail.com>
On Mon, Nov 12, 2012 at 06:41:05PM +0100, Felipe Contreras wrote:
> From: Ramkumar Ramachandra <artagnon@gmail.com>
>
> 'hg commit' fails otherwise in some versiosn of mercurial because of
s/versiosn/versions/
> +setup () {
> + (
> + echo "[ui]"
> + echo "username = A U Thor <author@example.com>"
> + ) >> "$HOME"/.hgrc
> +}
This makes sense, but I wonder if we should use something different from
the git author ident set up by the test scripts, just to double check
that we do not have any bugs in confusing the two during the import.
Something like "H G Wells <wells@example.com>" would work, and satisfies
my deep-seated desire for bad puns.
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] git-sh-setup: refactor ident-parsing functions
From: Junio C Hamano @ 2012-11-12 20:32 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, Ilya Basin, git
In-Reply-To: <20121112201235.GA7210@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Nov 12, 2012 at 12:08:37PM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>> > Changing the interface for get_author_ident_from_commit would be a pain,
>> > but if we just wanted to help filter-branch, we could do something like
>> > this:
>>
>> Yes, that is the direction I was alluding to.
>>
>> Callers of get_author_ident_from_commit can also do the same and
>> avoid rebuilding the same $pick_author_script over and over again,
>> or get_author_ident_from_commit can do so for its callers.
>
> I don't think get_author_ident_from_commit can do so on demand due to
> the subshell issue I mentioned.
OK, that is what I obviously missed.
> I wonder if we should simply generate these at build time and store them
> in the script.
Yeah, my knee-jerk reaction was that it was wasteful to run script
at runtime to build a constant string, but the engineering cost may
not be worth it.
^ permalink raw reply
* Re: [BUG] gitweb: XSS vulnerability of RSS feed
From: Junio C Hamano @ 2012-11-12 20:36 UTC (permalink / raw)
To: Jeff King; +Cc: Drew Northup, glpk xypron, git, jnareb
In-Reply-To: <20121112202701.GE4623@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Nov 12, 2012 at 03:24:13PM -0500, Jeff King wrote:
>
>> I think the right answer is going to be a well-placed call to esc_html.
>
> I'm guessing the right answer is this:
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 10ed9e5..a51a8ba 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -8055,6 +8055,7 @@ sub git_feed {
> $feed_type = 'history';
> }
> $title .= " $feed_type";
> + $title = esc_html($title);
> my $descr = git_get_project_description($project);
> if (defined $descr) {
> $descr = esc_html($descr);
>
> but I did not test it (and I am not that familiar with gitweb, so it is
> a slight guess from spending 5 minutes grepping and reading).
Yeah, that looks correct, given the way how the other variables
emitted with the same "print" like $descr and $owner are formed.
^ permalink raw reply
* Re: [PATCHv3] replace: parse revision argument for -d
From: Jeff King @ 2012-11-12 20:42 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Junio C Hamano
In-Reply-To: <a35a8f44b908bded0b475b02e7917011fb3bf90b.1352728712.git.git@drmicha.warpmail.net>
On Mon, Nov 12, 2012 at 03:18:02PM +0100, Michael J Gruber wrote:
> 'git replace' parses the revision arguments when it creates replacements
> (so that a sha1 can be abbreviated, e.g.) but not when deleting
> replacements.
>
> Make it parse the argument to 'replace -d' in the same way.
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
>
> Notes:
> v3 safeguards the hex buffer against reuse
Thanks, I don't see any other functional problems.
> diff --git a/builtin/replace.c b/builtin/replace.c
> index e3aaf70..33e6ec3 100644
> --- a/builtin/replace.c
> +++ b/builtin/replace.c
> @@ -46,24 +46,28 @@ typedef int (*each_replace_name_fn)(const char *name, const char *ref,
>
> static int for_each_replace_name(const char **argv, each_replace_name_fn fn)
> {
> - const char **p;
> + const char **p, *q;
I find this readable today, but I wonder if in six months we will wonder
what in the world "q" means. Maybe "short_refname" or something would be
appropriate?
-Peff
^ permalink raw reply
* Re: RFD: fast-import is picky with author names (and maybe it should - but how much so?)
From: Felipe Contreras @ 2012-11-12 20:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: gitzilla, Michael J Gruber, Git Mailing List, Jeff King
In-Reply-To: <7vwqxqiul3.fsf@alter.siamese.dyndns.org>
On Mon, Nov 12, 2012 at 6:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
> A Large Angry SCM <gitzilla@gmail.com> writes:
>
>> On 11/11/2012 07:41 AM, Felipe Contreras wrote:
>>> On Sat, Nov 10, 2012 at 8:25 PM, A Large Angry SCM<gitzilla@gmail.com> wrote:
>>>> On 11/10/2012 01:43 PM, Felipe Contreras wrote:
>>>
>>>>> So, the options are:
>>>>>
>>>>> a) Leave the name conversion to the export tools, and when they miss
>>>>> some weird corner case, like 'Author<email', let the user face the
>>>>> consequences, perhaps after an hour of the process.
>>>>>
>>>>> We know there are sources of data that don't have git-formatted author
>>>>> names, so we know every tool out there must do this checking.
>>>>>
>>>>> In addition to that, let the export tool decide what to do when one of
>>>>> these bad names appear, which in many cases probably means do nothing,
>>>>> so the user would not even see that such a bad name was there, which
>>>>> might not be what they want.
>>>>>
>>>>> b) Do the name conversion in fast-import itself, perhaps optionally,
>>>>> so if a tool missed some weird corner case, the user does not have to
>>>>> face the consequences.
>>>>>
>>>>> The tool writers don't have to worry about this, so we would not have
>>>>> tools out there doing a half-assed job of this.
>>>>>
>>>>> And what happens when such bad names end up being consistent: warning,
>>>>> a scaffold mapping of bad names, etc.
>>>>>
>>>>>
>>>>> One is bad for the users, and the tools writers, only disadvantages,
>>>>> the other is good for the users and the tools writers, only
>>>>> advantages.
>>>>>
>>>>
>>>> c) Do the name conversion, and whatever other cleanup and manipulations
>>>> you're interesting in, in a filter between the exporter and git-fast-import.
>>>
>>> Such a filter would probably be quite complicated, and would decrease
>>> performance.
>>>
>>
>> Really?
>>
>> The fast import stream protocol is pretty simple. All the filter
>> really needs to do is pass through everything that isn't a 'commit'
>> command. And for the 'commit' command, it only needs to do something
>> with the 'author' and 'committer' lines; passing through everything
>> else.
>>
>> I agree that an additional filter _may_ decrease performance somewhat
>> if you are already CPU constrained. But I suspect that the effect
>> would be negligible compared to the all of the SHA-1 calculations.
>
> More importantly, which do users prefer: quickly produce an
> incorrect result, or spend some more time to get it right?
Why not both?
If I do 'git clone hg::http://selenic.com/hg' I expect it to work, no
matter what. Then, if I care about getting it right, like for example
if the project is moving to git, then check
.git/hg/origin/bad-authors, and fill them with the right ones.
Of course, the current remote helper framework doesn't have the option
to map authors, but it could be added. That would be better than
letting every remote helper tool to have a custom way of mapping
authors, and also custom configuration for them.
> Because the exporting tool has a lot more intimate knowledge about
> how the names are represented in the history of the original SCM,
> canonicalization of the names, if done at that point, would likely
> to give us more useful results, than a canonicalization done at the
> beginning of the importer, which lacks SCM specific details. So in
> that sense, (a) is more preferrable than (b).
But it doesn't have more intimate knowledge. It has exactly the same
information as fast-import; nothing.
What intimate knowledge is a tool expected to get from this?
% hg commit -u 'Foo Bar<foo.bar@example.com> <none@none>' -m one
% hg --debug log
changeset: 0:5ef37a2c773f02d0e01f1ecdcc59149832d294e8
tag: tip
phase: draft
parent: -1:0000000000000000000000000000000000000000
parent: -1:0000000000000000000000000000000000000000
manifest: 0:c6d4cd25b9fc2f83b0dd51f4acbea9486fce54d7
user: Foo Bar<foo.bar@example.com> <none@none>
date: Sun Nov 11 18:33:00 2012 +0100
files+: file
extra: branch=default
description:
one
Some tools might, but if they did, then bad authors wouldn't be a problem.
> On the other hand, we would want consistency across the converted
> results no matter what SCM the history was originally in. E.g. a
> name without email that came from CVS or SVN would consistently want
> to become "name <noname@noname>" or "name <name>" or whatever, and
> letting exporting tools responsible for the canonicalization will
> lead them to create their own garbage. In that sense, (b) can be
> better than (a).
Or 'Unknown <unknown>' or '<none@none>' or '<>', or any of the forms
conversion tools have been doing for ages.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] Update cygwin.c for new mingw-64 win32 api headers
From: Jeff King @ 2012-11-12 20:58 UTC (permalink / raw)
To: Mark Levedahl; +Cc: git
In-Reply-To: <1352679440-4098-1-git-send-email-mlevedahl@gmail.com>
On Sun, Nov 11, 2012 at 07:17:20PM -0500, Mark Levedahl wrote:
> The cygwin project recently switched to a new implementation of the
> windows api, now using header files from the mingw-64 project. These
> new header files are incompatible with the way cygwin.c included the
> old headers: cygwin.c can be compiled using the new or the older (mingw)
> headers, but different files must be included in different order for each
> to work. The new headers are in use only for the current release series
> (based upon the v1.7.x dll version). The previous release series using
> the v1.5 dll is kept available but unmaintained for use on older versions
> of Windows. So, patch cygwin.c to use the new include ordering only if
> the dll version is 1.7 or higher.
I have very little knowledge of cygwin compatibility, so thanks for a
nice explanation. I'll queue it in 'pu' for now, and hopefully we can
get some test reports from other cygwin folks (on new and old cygwin).
> diff --git a/Makefile b/Makefile
> index f69979e..1cc5d96 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1082,6 +1082,7 @@ ifeq ($(uname_O),Cygwin)
> NO_SYMLINK_HEAD = YesPlease
> NO_IPV6 = YesPlease
> OLD_ICONV = UnfortunatelyYes
> + V15_MINGW_HEADERS = YesPlease
> endif
The "if" part of the conditional that did not make it into the context
above is an expr match for "1.6.*" From the name, I would think that we
would want to use these headers on cygwin 1.5.* , too. Is v1.5 too old
to care about now?
-Peff
^ permalink raw reply
* Re: [BUG] gitweb: XSS vulnerability of RSS feed
From: Jakub Narębski @ 2012-11-12 21:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Drew Northup, glpk xypron, git
In-Reply-To: <7vmwymh83r.fsf@alter.siamese.dyndns.org>
On Mon, Nov 12, 2012 at 9:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>> On Mon, Nov 12, 2012 at 03:24:13PM -0500, Jeff King wrote:
>>
>>> I think the right answer is going to be a well-placed call to esc_html.
>>
>> I'm guessing the right answer is this:
>>
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 10ed9e5..a51a8ba 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -8055,6 +8055,7 @@ sub git_feed {
>> $feed_type = 'history';
>> }
>> $title .= " $feed_type";
>> + $title = esc_html($title);
>> my $descr = git_get_project_description($project);
>> if (defined $descr) {
>> $descr = esc_html($descr);
>>
>> but I did not test it (and I am not that familiar with gitweb, so it is
>> a slight guess from spending 5 minutes grepping and reading).
>
> Yeah, that looks correct, given the way how the other variables
> emitted with the same "print" like $descr and $owner are formed.
It looks like good solution to me too.
Nb. the problems with feed are mainly because it is generated
by hand even more than HTML (which uses CGI.pm).
--
Jakub Narębski
--
Jakub Narebski
^ permalink raw reply
* Re: [RFC PATCH 1/1] status: Allow for short-form via config option
From: Jeff King @ 2012-11-12 21:17 UTC (permalink / raw)
To: Thomas Adam; +Cc: git
In-Reply-To: <1352674383-23654-2-git-send-email-thomas@xteddy.org>
On Sun, Nov 11, 2012 at 10:53:03PM +0000, Thomas Adam wrote:
> It is currently not possible to use the short-form output of git status
> without declaring an alias to do so.
>
> This isn't always desirable therfore, define a git config option which can
> be set to display the short-form: status.shortwithbranch
We usually try to avoid booleans for selection among options, even if
there is currently only one useful option. That way, it makes it easier
to extend later when another option presents itself. So having
"status.format" that you could set to "short", "long", or "porcelain"
would make more sense (although "short" is likely to be the only useful
one currently).
And then we can have a separate boolean like "status.branch" to show the
branch by default when showing short format. That would would naturally
extend to more booleans as other options are added (Phil Hord's recent
"show tree state" patches come to mind).
We also need to consider the impact of options on scripts. I think
status.format should be OK, since any script calling "git status" would
have to explicitly pass "--porcelain" already, which would override this
option. But we would want to make sure that "status.branch" would not
affect the porcelain.
> ---
> builtin/commit.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
It would need documentation, and tests would be good both to make sure
the feature works, but also to demonstrate that it does not break
--porcelain.
> diff --git a/builtin/commit.c b/builtin/commit.c
> index a17a5df..552a9f1 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1142,6 +1142,18 @@ static int git_status_config(const char *k, const char *v, void *cb)
> return error(_("Invalid untracked files mode '%s'"), v);
> return 0;
> }
> +
> + if (!strcmp(k, "status.shortwithbranch")) {
> + if (git_config_bool(k, v)) {
> + if (!v)
> + return config_error_nonbool(k);
> + else if(!strcmp(v, "true")) {
> + status_format = STATUS_FORMAT_SHORT;
> + s->show_branch = 1;
> + }
> + return 0;
> + }
> + }
I'm not sure what is going on with the extra nonbool and "true" check.
Shouldn't it just be:
if (git_config_bool(k, v)) {
status_format = STATUS_FORMAT_SHORT;
s->show_branch = 1;
}
else {
/* what do we do when it is false? */
}
return 0;
If we follow my suggestions above, then it would be more like:
if (!strcmp(k, "status.format")) {
if (!v)
return config_error_nonbool(k);
return parse_status_format(v, &status_format);
}
if (!strcmp(k, "status.branch")) {
s->show_branch = git_config_bool(k, v);
return 0;
}
but that would still have to resolve the setting of show_branch when
--porcelain is in use. I think you would need to store the
config-derived value separately, and then only fill it into
s->show_branch if no value was given on the command-line _and_ we are
not showing the porcelain format.
-Peff
^ permalink raw reply
* Re: [BUG] gitweb: XSS vulnerability of RSS feed
From: Jeff King @ 2012-11-12 21:34 UTC (permalink / raw)
To: Jakub Narębski; +Cc: Junio C Hamano, Drew Northup, glpk xypron, git
In-Reply-To: <CANQwDwdRTeaVS5cMic5gv9SP1A8Z1vruOsZBFfMDQDTZHBAtvQ@mail.gmail.com>
On Mon, Nov 12, 2012 at 10:13:27PM +0100, Jakub Narębski wrote:
> > Yeah, that looks correct, given the way how the other variables
> > emitted with the same "print" like $descr and $owner are formed.
>
> It looks like good solution to me too.
>
> Nb. the problems with feed are mainly because it is generated
> by hand even more than HTML (which uses CGI.pm).
Yeah, I noticed that. Here it is in patch form with a test. It would be
nice if people interested in gitweb would add more entries to the XSS
test below (I put in the one that fails, along with an obvious variation
that is actually OK).
I didn't look carefully through the rest of gitweb for more XSS
instances. From a glance, it looks like we mostly use the safe CGI
methods, but probably it could use a full audit (which again, I would be
happy if people who care more about gitweb would do).
-- >8 --
Subject: [PATCH] gitweb: escape html in rss title
The title of an RSS feed is generated from many components,
including the filename provided as a query parameter, but we
failed to quote it. Besides showing the wrong output, this
is a vector for XSS attacks.
Signed-off-by: Jeff King <peff@peff.net>
---
gitweb/gitweb.perl | 1 +
t/t9502-gitweb-standalone-parse-output.sh | 15 +++++++++++++++
2 files changed, 16 insertions(+)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 10ed9e5..a51a8ba 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -8055,6 +8055,7 @@ sub git_feed {
$feed_type = 'history';
}
$title .= " $feed_type";
+ $title = esc_html($title);
my $descr = git_get_project_description($project);
if (defined $descr) {
$descr = esc_html($descr);
diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh
index 731e64c..3a8e7d3 100755
--- a/t/t9502-gitweb-standalone-parse-output.sh
+++ b/t/t9502-gitweb-standalone-parse-output.sh
@@ -185,5 +185,20 @@ test_expect_success 'forks: project_index lists all projects (incl. forks)' '
test_cmp expected actual
'
+xss() {
+ echo >&2 "Checking $1..." &&
+ gitweb_run "$1" &&
+ if grep "$TAG" gitweb.body; then
+ echo >&2 "xss: $TAG should have been quoted in output"
+ return 1
+ fi
+ return 0
+}
+
+test_expect_success 'xss checks' '
+ TAG="<magic-xss-tag>" &&
+ xss "a=rss&p=$TAG" &&
+ xss "a=rss&p=foo.git&f=$TAG"
+'
test_done
--
1.8.0.207.gdf2154c
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox