Re: [PATCH ALTERNATIVE v6.v3 4/6] config: parse http.<url>.<variable> using urlmatch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Aug 5, 2013, at 15:56, Junio C Hamano wrote:
"Kyle J. McKay" <mackyle@xxxxxxxxx> writes:

Use the urlmatch_config_entry() to wrap the underlying
http_options() two-level variable parser in order to set
http.<variable> to the value with the most specific URL in the
configuration.

Signed-off-by: Jeff King <peff@xxxxxxxx>
Signed-off-by: Kyle J. McKay <mackyle@xxxxxxxxx>
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---

Oops, what did we sign-off?

Some code removal.  No new additions.  Actually this:

On Aug 1, 2013, at 14:44, Junio C Hamano wrote:

* jc/url-match (2013-07-31) 6 commits
- config: "git config --get-urlmatch" parses section.<url>.key
- builtin/config: refactor collect_config()
- config: parse http.<url>.<variable> using urlmatch
- config: add generic callback wrapper to parse section.<url>.key
- config: add helper to normalize and match URLs
- http.c: fix parsing of http.sslCertPasswordProtected variable

Reroll of km/http-curl-config-per-url topic.  Peff raises many good
points about the tests for http.* variables.

Waiting for the discussion to conclude, hopefully with a replacement test.

As requested.

This version of 4/6 moves the tests to t0110 since urlmatch is now global. The config tests are removed since part 6/6 already has those and they no
longer belong with the urlmatch normalization tests.

The Makefile rule has been removed since it's no longer needed to build
correctly as the test program no longer includes http.c.

Other than those changes (and a minor rename to reflect the new location),
this patch is identical to the previous v6.v2 4/6.

Ahh, figures.  Thanks.

The remaining tests, by the way, have not changed. They are identical to previous versions.

Peff, any comments?

diff --git a/t/t0110-urlmatch-normalization.sh b/t/t0110-urlmatch- normalization.sh
new file mode 100755
index 00000000..8d6096d4
--- /dev/null
+++ b/t/t0110-urlmatch-normalization.sh
@@ -0,0 +1,177 @@
+#!/bin/sh
+
+test_description='urlmatch URL normalization'
+. ./test-lib.sh
+
+# The base name of the test url files
+tu="$TEST_DIRECTORY/t0110/url"
+
+# Note that only file: URLs should be allowed without a host

It is somewhat unfortunate that the form most commonly used for
pushing is not supported at all, i.e.

	host:path

That is an SSH extension and they are certainly not URLs according to RFC 3986 because that would require every host to be its own scheme.

Also, host:path cannot in the general case, be unambiguously translated to a URL.

For example, repo.or.cz:srv/git/alt-git, has no translation. It is different from repo.or.cz:/srv/git/alt-git which does have a translation. There's no guarantee that inserting a '/' will not change the meaning of the URL (that only happens to be the case on repo.or.cz because all the ssh git users in the chroot jail have a '/' home directory).

Current configuration set may not have anything interesting to
affect the git-over-ssh push codepath, so in practice it may not
matter, though.

+test_expect_success 'url authority' '

"authority" refers to the host part? (not a complaint, but is a
question)

It refers to this production from RFC 3986 Section "3.2 Authority":

authority = [ userinfo "@" ] host [ ":" port ]

+test_expect_success 'url port checks' '
+	test-urlmatch-normalization "xyz://q@xxxxxxxxx:" &&

This is presumably replaced by a default port for xyz:// scheme,
whatever the default port is, in other words, it is as if no colon
is given at the end?

Yes.

The "port" production above is:

port = *DIGIT

which means 0 or more digits.

+	test-urlmatch-normalization "xyz://q@xxxxxxxxx:456/" &&
+	! test-urlmatch-normalization "xyz://q@xxxxxxxxx:0" &&
+	! test-urlmatch-normalization "xyz://q@xxxxxxxxx:0000000" &&

Port #0 is disallowed?

Intentionally so.

The comments from urlmatch.c talk about this:

/*
 * Port number must be all digits with leading 0s removed
 * and since all the protocols we deal with have a 16-bit
 * port number it must also be in the range 1..65535
 * 0 is not allowed because that means "next available"
 * on just about every system and therefore cannot be used
 */

+	test-urlmatch-normalization "xyz://q@xxxxxxxxx:0000001?" &&

Is it the same as specifying "xyz://q@xxxxxxxxx:1?" and does it
match "xyz://q@xxxxxxxxx:1"?

+	test-urlmatch-normalization "xyz://q@xxxxxxxxx:065535#" &&

Ditto, for 65535 and without #-fragment at the end?

+test_expect_success 'url port normalization' '
+ test "$(test-urlmatch-normalization -p "http://x:800";)" = "http:// x:800/" && + test "$(test-urlmatch-normalization -p "http://x:0800";)" = "http://x:800/"; && + test "$(test-urlmatch-normalization -p "http://x:00000800";)" = "http://x:800/"; && + test "$(test-urlmatch-normalization -p "http://x:065535";)" = "http://x:65535/"; && + test "$(test-urlmatch-normalization -p "http://x:1";)" = "http://x: 1/" && + test "$(test-urlmatch-normalization -p "http://x:80";)" = "http:// x/" && + test "$(test-urlmatch-normalization -p "http://x:080";)" = "http:// x/" && + test "$(test-urlmatch-normalization -p "http://x:000000080";)" = "http://x/"; && + test "$(test-urlmatch-normalization -p "https://x:443";)" = "https://x/ " && + test "$(test-urlmatch-normalization -p "https://x:0443";)" = "https://x/ " && + test "$(test-urlmatch-normalization -p "https://x:000000443";)" = "https://x/ "
+'

OK, these answer most of the previous questions.

+# http://@foo specifies an empty user name but does not specify a password
+# http://foo  specifies neither a user name nor a password
+# So they should not be equivalent
+test_expect_success 'url equivalents' '
+	test-urlmatch-normalization "httP://x" "Http://X/" &&
+ test-urlmatch-normalization "Http://%4d%65:%4d^%70@xxxxxxxx" "hTTP://Me :%4D^p@xxxxxxxx:80/" && + ! test-urlmatch-normalization "https://@x.y/^"; "httpS://x.y:443/ ^" &&

The comment is about this test, which seems to make sense.  What is
"^"?  Just a random valid character that can appear in the path?
(not a complaint, but is a question).

The character '^' is one of the always-unsafe characters that must always be escaped. It's also one of the always-unsafe characters that's easy to include in the tests as it doesn't require escaping or backslashing or binary includes. It doesn't otherwise have any special meaning.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]