From dev-return-40223-archive-asf-public=cust-asf.ponee.io@subversion.apache.org Thu Jan 30 19:43:09 2020 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 4B2E218062B for ; Thu, 30 Jan 2020 20:43:09 +0100 (CET) Received: (qmail 7405 invoked by uid 500); 30 Jan 2020 19:43:08 -0000 Mailing-List: contact dev-help@subversion.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list dev@subversion.apache.org Received: (qmail 7393 invoked by uid 99); 30 Jan 2020 19:43:08 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd1-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 30 Jan 2020 19:43:08 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd1-us-west.apache.org (ASF Mail Server at spamd1-us-west.apache.org) with ESMTP id 954DDC0AC7 for ; Thu, 30 Jan 2020 19:43:07 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 1.649 X-Spam-Level: * X-Spam-Status: No, score=1.649 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, JMQ_SPF_NEUTRAL=0.5, KAM_ASCII_DIVIDERS=0.8, KAM_EU=0.5, KAM_INFOUSMEBIZ=0.75, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=disabled Authentication-Results: spamd1-us-west.apache.org (amavisd-new); dkim=pass (2048-bit key) header.d=foad.me.uk header.b=D683vlfh; dkim=pass (2048-bit key) header.d=messagingengine.com header.b=c9+OYKD9 Received: from mx1-ec2-va.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id QyvwbCTYZMHS for ; Thu, 30 Jan 2020 19:43:05 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=66.111.4.27; helo=out3-smtp.messagingengine.com; envelope-from=julian@foad.me.uk; receiver= Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by mx1-ec2-va.apache.org (ASF Mail Server at mx1-ec2-va.apache.org) with ESMTPS id A5E27BB803 for ; Thu, 30 Jan 2020 19:43:05 +0000 (UTC) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 5A26421B2B; Thu, 30 Jan 2020 14:42:59 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Thu, 30 Jan 2020 14:42:59 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=foad.me.uk; h= date:from:to:cc:message-id:in-reply-to:references:subject :mime-version:content-type:content-transfer-encoding; s=fm3; bh= 5OIRp1XkEcl/urx4p1zTTUP+DXCODYkuv3D7wJvwGoA=; b=D683vlfh/q3JGSc7 7wc7+ptdDALKDzsNfX/UNOBxvods79eIvkR0Ni4u4QcjFjjL9aU4mEo5EBQGbQmD Mmlwl7ta5ycE2bxCF3RVRQQqht18wpBb3tflqpcJtPG7I24+V2W/kbZgeWqM/ss5 ziCdE1hdVN7HQyQfeTdIIowaZc1pVf5m95VvqABYaeBYOEvP3JPf5Uv4hSpaeMAU 5ho1frD+gNm4G4JY20+5l+/IBO5btKO7MhfIswBq9OWJFByAGg1z6JJCYCPh2TfU +2s0MJ+b/aH6gEvlukXNDQNxUPtbSM4/42VvXpmTOemBXxyMP/MMJlfX4WWP4CzM cSmRCA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=5OIRp1XkEcl/urx4p1zTTUP+DXCODYkuv3D7wJvwG oA=; b=c9+OYKD9H+QZw9+zV+L/9FtgHZPNQu7FmL5Xfgv+FUBueJ48GHxIo+P+Y R+XqWGFRXZkSd3QxG4LWGKmVQ5Apz4Dal6RSkw/LkWZK725o2XrBh1fcNTOpg3Dh rYcGueWV7Oj9icSjp0dISuEzxyK4tYUgyv7fCxWfG92FZrmS7rKvAq+aET2T7Cvo XgYLBP9s2n8Y5xeEjjTRDYYEeRtdVxF9Yu8ikjQjD/nm/GR0iJx5xWRK4Ch+i5QI /0D9HVcobW4VbcSGfoeCT3indZQt3OaiJP/N+quAWz2S2erO1gRyeYsM23nv4j0D pVRVPXj4hnp1jVCUOEM1gAjxDqBpw== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedugedrfeekgdduvdeiucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffkjghfufggtgfgsehtjeertddttdejnecuhfhrohhmpefluhhlihgr nhcuhfhorgguuceojhhulhhirghnsehfohgrugdrmhgvrdhukheqnecuffhomhgrihhnpe hfrghirhgtohguvgdrvghupdgvgigrmhhplhgvrdgtohhmpdgrphgrtghhvgdrohhrghen ucfkphepkedurddujeegrdduheelrddvvdeknecuvehluhhsthgvrhfuihiivgeptdenuc frrghrrghmpehmrghilhhfrhhomhepjhhulhhirghnsehfohgrugdrmhgvrdhukh X-ME-Proxy: Received: from [127.0.0.1] (unknown [81.174.159.228]) by mail.messagingengine.com (Postfix) with ESMTPA id 6AD58328005E; Thu, 30 Jan 2020 14:42:58 -0500 (EST) Date: Thu, 30 Jan 2020 19:43:00 +0000 (UTC) From: Julian Foad To: Stefan Sperling Cc: dev@subversion.apache.org Message-ID: In-Reply-To: <20200130193419.GH19242@byrne.stsp.name> References: <20200130193419.GH19242@byrne.stsp.name> Subject: Re: ra_serf redirect URL canonicalization fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Correlation-ID: Meh... What a mess. One part of the clean up: we should make very clear where a URL variable holds a 'canonical' URL (in this conversation, that means according to svn's own cannonicalisation rules) and where it does not. As our general rule is URLs in Subversion source code should be canonical, the way to make this clear is to label any non-canonical URL as such, by it's variable name, if it exists for more than about one line of code from where it's received to where it's validated/canonicalized Sent with FairEmail [https://email.faircode.eu/] , an open source, privacy friendly email app for Android 30 Jan 2020 19:34:42 Stefan Sperling : > I managed to trigger a client-side assertion failure today with a > simple HTTP redirect. I have fixed this issue in r1873375 (see below). > > When I tried to merge this fix to 1.10.x I learned that this behaviour > was intentionally changed already in r1866899. We used to canonicalize > these URLs, but then we stopped doing so because we could end up > misreporting an unrelated error condition (see below). > > My patch doesn't cleanly merge to 1.10.x because svn_uri_canonicalize_safe() > does not yet exist there. Should we backport it to 1.10.x as a private > function, or revert r1866899 from 1.10.x, or do we leave 1.10.x as is? > > To me it looks as if the consequences of r1866899 weren't fully considered. > The server should not be able to trigger assertions in the client's libraries. > > Would svn_uri_canonicalize_safe() allow for detecting a redirect loop problem? > My patch is not making use of the non-canonicalized version of the URL but > this version remains available when svn_uri_canonicalize_safe() is used. > Perhaps we could somehow take advantage of that to fix both problems? > > ------------------------------------------------------------------------ > r1873375 | stsp | 2020-01-30 20:14:49 +0100 (Thu, 30 Jan 2020) | 16 lines > > Canonicalize redirect URLs in ra_serf, rather than using them as-is. > This prevents an assertion failure in the client if the server sends > a redirect to a non-canonical URL. > > If Apache HTTPD uses a redirect statement such as this: > Redirect permanent "/svn" https://svn.example.com/svn/ > then the redirect URL won't be canonical. For example, access to the path > "/svn/trunk" will be redirected to https://svn.example.com/svn//trunk > > Note the double-slash which eventually triggers an assertion failure when > the redirect URL gets checked at an API boundary outside of ra_serf. > > * subversion/libsvn_ra_serf/options.c > (svn_ra_serf__exchange_capabilities): Treat redirect URLs as untrusted > input and attempt to canonicalize them. > > ------------------------------------------------------------------------ > r1866899 | julianfoad | 2019-09-13 14:24:01 +0200 (Fri, 13 Sep 2019) | 26 lines > > When following an HTTP redirect, use the Location header URL exactly. > > Previously we canonicalized the redirect URL, which could lead to a redirect > loop. Then Subversion would report a redirect loop as the error, potentially > hiding a more interesting error such as when the target is not in fact a > Subversion repository. > > A manual test case (on a non-repository): > before: > $ svn ls https://archive.apache.org/dist > Redirecting to URL 'https://archive.apache.org/dist': > Redirecting to URL 'https://archive.apache.org/dist': > svn: E195019: Redirect cycle detected for URL 'https://archive.apache.org/dist' > > after: > $ svn ls https://archive.apache.org/dist > Redirecting to URL 'https://archive.apache.org/dist/': > svn: E170013: Unable to connect to a repository at URL 'https://archive.apache.org/dist/' > svn: E175003: The server at 'https://archive.apache.org/dist/' does not support the HTTP/DAV protocol > > * subversion/libsvn_ra_serf/options.c > (svn_ra_serf__exchange_capabilities): Don't canonicalize the redirect URL. > > * subversion/libsvn_ra_serf/util.c > (response_get_location): Don't canonicalize the redirect URL. > > ------------------------------------------------------------------------ >