From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x632.google.com (mail-ej1-x632.google.com [IPv6:2a00:1450:4864:20::632]) by sourceware.org (Postfix) with ESMTPS id C8C683851C12 for ; Fri, 29 May 2020 04:39:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C8C683851C12 Received: by mail-ej1-x632.google.com with SMTP id l27so767448ejc.1 for ; Thu, 28 May 2020 21:39:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:references:message-id:date :mime-version:in-reply-to:content-language; bh=x0SsOzVE/3/wwkL2SxTXyxkn1rw2n55cmH54d8hMXYM=; b=ifr1keFtsMrs+daogqNvG8siCF3NVYUWKWDmH51DUYoZyKXjmOue1KLO8nDJndGIcO B5PQ8remcDt+3VLjt3WTgieepgvgWdWtE+Oa2UiOj64v8PtdY1/ngZ5+a25SPAEhXmkO 8xcAGFKBKl2Tc2Cdr+W86o/sOnBQDp4DpVznLPyszCwClsZvhlyz+7CNI38OngU5agvD V4KJh1zEHJ5CXXDp47OQsDzfIwfEDUkaKP1ZS/J/ccL8ycamJc6oMK+3q29G753X9MRG KsPljfIz6n7gbzzjR4H+U+eLDP3WK8FczX/Ah41kIg9608MU44FXoWQDx3aWH70tkiHj NSrw== X-Gm-Message-State: AOAM530Zn6IqU/G9+9pe+x9HdyKjSHAUY9ias084PQ8ZPrpozeXUse36 yAWsfgTcPCDyYbtWd4Mgmbz0OLU2 X-Google-Smtp-Source: ABdhPJxVhdvNDGjs1UYcEf7gWUcSMw7jxaJgOeVLPTa2dFl7R8SZLuBNTzdFoxpStY5QG2x50M2ngQ== X-Received: by 2002:a17:906:44f:: with SMTP id e15mr5749482eja.161.1590727165544; Thu, 28 May 2020 21:39:25 -0700 (PDT) Received: from ?IPv6:2a02:8109:9f40:4576::6912? ([2a02:8109:9f40:4576::6912]) by smtp.gmail.com with ESMTPSA id j11sm208687ejk.67.2020.05.28.21.39.23 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 28 May 2020 21:39:24 -0700 (PDT) Subject: Re: cygport development From: Federico Kircheis To: cygwin-apps@cygwin.com References: <3501279f-70c8-042c-2e60-2cd315277ae9@SystematicSw.ab.ca> <97e449ba-2c23-a110-ce95-850394b398cf@gmail.com> <871rvgwqbc.fsf@Rainer.invalid> <0d57d59b-638f-47ae-bdd0-eefda2e63f0c@gmail.com> <7ac8dfe8-b7ae-8d4d-03aa-a8fbd95a00ef@gmail.com> <1e2bd9cc5346a540b1a355847af57476f6ccedab.camel@cygwin.com> Message-ID: <8726a1ec-c4cd-8c84-ccd1-316ebd2627c3@gmail.com> Date: Fri, 29 May 2020 06:38:53 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------5C71C4EDF045AE9AE67E56BC" Content-Language: en-US X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: cygwin-apps@cygwin.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Cygwin package maintainer discussion list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 May 2020 04:39:28 -0000 This is a multi-part message in MIME format. --------------5C71C4EDF045AE9AE67E56BC Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit I did not get any response to my last questions, so I hope this patch is enough. Any thought about my other arguments? Federico On 5/17/20 7:54 PM, Federico Kircheis wrote: > Thank you for the feedback. > >> This patch is clearly not limited to the protection of data, as it >> quotes variables that could in no way contain a space or have anything >> to do with file paths. > > Could you please point me to which variables are unrelated to files. > > AFAIK i quoted files and arguments, which can all contain whitespace. > > For example I did quote ${unpack_file_path}, but not ${unpack_cmd}. > >> As mentioned multiple times, using filenames >> ore directories with spaces is asking for trouble, and I have no >> interest in trying to support such a case. > > The first commit makes sure that no information is lost while processing > file with spaces or other characters that cause globbing. This prevents > writing or modifying the wrong files, which is what happened to me. > > The second commit add exit in case `cd` fails, which prevents other > errors afterwards. > > Those modification do not add support for path with whitespace, as I was > still unable to compile the software, they did however prevent cygport > to delete unrelated data (or create data in the wrong location). > > >> I'm willing to consider a >> *limited* patch that makes sure that cygport doesn't do something it >> shouldn't in that case, but that's about it. > > Also because if the underlying tool like `make` does not support spaces, > it has no benefit. > > The most minimal patch I can imagine is exiting if `cd` fails (just the > second commit). > Would you accept that? > But please also consider my other arguments. > >> Yaakov > > PS: > > A "nice" side-effect to quoting most variables that could contain white > space is that static-analyzers like shellcheck will emit less > diagnostic, making it easier to discover potential errors. > --------------5C71C4EDF045AE9AE67E56BC Content-Type: text/x-patch; charset=UTF-8; name="path-with-spaces.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="path-with-spaces.patch" >From 9dec371efa2f4f943bdd660618a0e1d91b6cfb4a Mon Sep 17 00:00:00 2001 From: Federico Kircheis Date: Tue, 2 Jul 2019 21:02:36 +0200 Subject: [PATCH] Exit in case `cd` fails --- lib/src_fetch.cygpart | 2 +- lib/src_prep.cygpart | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/src_fetch.cygpart b/lib/src_fetch.cygpart index a273045..acea3a6 100644 --- a/lib/src_fetch.cygpart +++ b/lib/src_fetch.cygpart @@ -156,7 +156,7 @@ __src_fetch() { done # the RCS_fetch functions change PWD - cd ${top}; + cd ${top} || error "Unable to cd to ${top}" for uri in ${SRC_URI} ${PATCH_URI} do diff --git a/lib/src_prep.cygpart b/lib/src_prep.cygpart index 80ba8d5..fb99bfd 100644 --- a/lib/src_prep.cygpart +++ b/lib/src_prep.cygpart @@ -189,7 +189,7 @@ __gpg_verify() { } __mkdirs() { - cd ${top}; + cd ${top} || error "Unable to cd to ${top}"; mkdir -p ${srcdir} ${origsrcdir} ${B} ${D} ${T} ${configdir} ${logdir} ${distdir} ${patchdir} ${spkgdir}; } @@ -286,7 +286,7 @@ __src_prep() { local tar_patch; local n=1; - cd ${top}; + cd ${top} || error "Unable to cd to ${top}"; __mkdirs; @@ -345,7 +345,7 @@ __src_prep() { __gpg_verify ${top}/${src_patchfile} "SOURCE PATCH"; fi - cd ${origsrcdir}; + cd ${origsrcdir} || error "Unable to cd to ${origsrcdir}"; for src_pkg in ${_src_orig_pkgs} do @@ -377,7 +377,7 @@ __src_prep() { # cd will fail if not executable (e.g. dot2tex) chmod +x ${origsrcdir}/${SRC_DIR}; - cd ${origsrcdir}/${SRC_DIR}; + cd ${origsrcdir}/${SRC_DIR} || error "Unable to cd to ${origsrcdir}/${SRC_DIR}"; #****v* Preparation/DISTCLEANFILES # DESCRIPTION @@ -404,7 +404,7 @@ __src_prep() { if __check_function src_unpack_hook then __check_unstable src_unpack_hook; - cd ${origsrcdir}/${SRC_DIR}; + cd ${origsrcdir}/${SRC_DIR} | error "Unable to cd to ${origsrcdir}/${SRC_DIR}"; fi for src_patch in ${_src_orig_patches} @@ -446,7 +446,7 @@ __src_prep() { if __check_function src_patch_hook then __check_unstable src_patch_hook; - cd ${origsrcdir}/${SRC_DIR}; + cd ${origsrcdir}/${SRC_DIR} || error "Unable to cd to ${origsrcdir}/${SRC_DIR}"; fi __step "Preparing working source directory"; @@ -456,7 +456,7 @@ __src_prep() { mkdir -p ${C}; ln -sfn ${C} ${workdir}/CYGWIN-PATCHES; - cd ${S}; + cd ${S} || error "Unable to cd to ${S}"; if [ -f ${top}/${cygwin_patchfile} ] then -- 2.26.2 --------------5C71C4EDF045AE9AE67E56BC--