git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: "Torsten Bögershausen" <tboegi@web.de>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH] is_hfs_dotgit: loosen over-eager match of \u{..47}
Date: Tue, 23 Dec 2014 13:18:12 -0500	[thread overview]
Message-ID: <20141223181812.GA27078@peff.net> (raw)
In-Reply-To: <54998949.9090908@web.de>

On Tue, Dec 23, 2014 at 04:24:57PM +0100, Torsten Bögershausen wrote:

> Don't we have the same possible problem under NTFS?
> Under Linux + VFAT ?
> Under all OS + VFAT ?

I'm not sure what you mean.

This code path is _only_ about checking for HFS+-specific problems. We
check general case-insensitivity in another code path. And we check
NTFS-specific problems in another code path.

(Technically we could make a "check all" function that runs over the
data only once, which would be more efficient. But doing it this way
makes the code much easier to follow).

> And would it make sense to turn this
> >   return (out & 0xffffff00) ? out : tolower(out);
> into this:
> static ucs_char_t unicode_tolower(ucs_char_t ch) {
>    return (ch & 0xffffff00) ? ch : tolower(ch);
> }

Perhaps, but you would need a big warning at the top of that function
that is _only_ downcasing ASCII characters. I.e., it is fine for our use
here, but you would not want other unicode-aware code to call it. The
next_hfs_char already has such a warning at the top.

> And what happens if I export NTFS to Mac OS X?
> (Other combinations possible)
> Shouldn't fsck under all OS warn for NTFS and hfs possible attacks ?

Fsck already warns for all system types, no matter what platform you're
on. And we do case-insensitive blocking of ".git" entering the index for
all platforms.

We don't turn on filesystem-specific blocking of paths entering the
index on all platforms. You get HFS+ blocking by default on OS X, and
NTFS on Windows. If you are using HFS+ on Linux, you should set
core.protectHFS.

Possibly we should just turn on all checks everywhere. That would be a
safer default, at the cost to Linux folks of:

  1. Some slight inefficiency in each read-tree, as we have to do extra
     processing on each name.

  2. Some names would be disallowed that are otherwise OK. For the most
     part these are not names that would be used in practice, though
     (e.g., losing the ability to create ".git\u200c" is not a big loss
     to anyone). I think Git for Windows has started blocking other
     stuff like "CON" that is not specifically related to .git, though,
     and that is more plausible for somebody to use in real life.

-Peff

  parent reply	other threads:[~2014-12-23 18:18 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-23  8:45 [PATCH] is_hfs_dotgit: loosen over-eager match of \u{..47} Jeff King
2014-12-23 15:24 ` Torsten Bögershausen
2014-12-23 18:17   ` Junio C Hamano
2014-12-23 18:18   ` Jeff King [this message]
2014-12-23 20:14 ` Jonathan Nieder
2014-12-23 21:02   ` Junio C Hamano
2014-12-23 21:12     ` Jeff King
2014-12-23 21:09   ` Jeff King
2014-12-23 20:31 ` Johannes Sixt
2014-12-23 21:11   ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141223181812.GA27078@peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=tboegi@web.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).