From e014c981f2c7ae05db42276933ee5e3fb4bff44b Mon Sep 17 00:00:00 2001 From: Himadri Bhattacharjee <107522312+lavafroth@users.noreply.github.com> Date: Wed, 17 Jan 2024 12:11:47 +0530 Subject: [PATCH] Disallow background operator before && or || Co-authored-by: Johannes Altmanninger Closes #10228 Fixes #9911 --- src/parse_util.rs | 38 +++++++++++++++++++++++++++++++++++--- src/tests/parse_util.rs | 14 ++++++++++++-- src/wchar_ext.rs | 19 +++++++++++++++++++ 3 files changed, 66 insertions(+), 5 deletions(-) diff --git a/src/parse_util.rs b/src/parse_util.rs index 6e0b9f34b..eb6969ef6 100644 --- a/src/parse_util.rs +++ b/src/parse_util.rs @@ -1,5 +1,5 @@ //! Various mostly unrelated utility functions related to parsing, loading and evaluating fish code. -use crate::ast::{self, Ast, Keyword, Leaf, List, Node, NodeVisitor}; +use crate::ast::{self, Ast, Keyword, Leaf, List, Node, NodeVisitor, Token}; use crate::builtins::shared::builtin_exists; use crate::common::{ escape_string, unescape_string, valid_var_name, valid_var_name_char, EscapeFlags, @@ -29,7 +29,7 @@ use crate::wchar::prelude::*; use crate::wcstringutil::count_newlines; use crate::wcstringutil::truncate; use crate::wildcard::{ANY_CHAR, ANY_STRING, ANY_STRING_RECURSIVE}; -use std::ops; +use std::{iter, ops}; /// Handles slices: the square brackets in an expression like $foo[5..4] /// \return the length of the slice starting at \p in, or 0 if there is no slice, or None on error. @@ -1114,6 +1114,8 @@ pub fn parse_util_detect_errors_in_ast( if jc.pipe.has_source() && jc.statement.try_source_range().is_none() { has_unclosed_pipe = true; } + } else if let Some(job_conjunction) = node.as_job_conjunction() { + errored |= detect_errors_in_job_conjunction(job_conjunction, &mut out_errors); } else if let Some(jcc) = node.as_job_conjunction_continuation() { // Somewhat clumsy way of checking for a job without source in a conjunction. // See if our conjunction operator (&& or ||) has source but our job does not. @@ -1392,6 +1394,36 @@ pub fn parse_util_detect_errors_in_argument( } } +fn detect_errors_in_job_conjunction( + job_conjunction: &ast::JobConjunction, + parse_errors: &mut Option<&mut ParseErrorList>, +) -> bool { + // Disallow background immediately before conjunction continuations. For example: + // foo & && bar + // foo & || baz + let continuations = &job_conjunction.continuations; + let jobs = iter::once(&job_conjunction.job) + .chain(continuations.iter().map(|continuation| &continuation.job)); + for (job, continuation) in jobs.zip(continuations.iter()) { + if job.bg.is_some() { + let conjunction = &continuation.conjunction; + return append_syntax_error!( + parse_errors, + conjunction.source_range().start(), + conjunction.source_range().length(), + BOOL_AFTER_BACKGROUND_ERROR_MSG, + if conjunction.token_type() == ParseTokenType::andand { + L!("&&") + } else { + L!("||") + } + ); + } + } + + false +} + /// Given that the job given by node should be backgrounded, return true if we detect any errors. fn detect_errors_in_backgrounded_job( job: &ast::JobPipeline, @@ -1778,7 +1810,7 @@ pub fn parse_util_expand_variable_error( } /// Error message for use of backgrounded commands before and/or. -const BOOL_AFTER_BACKGROUND_ERROR_MSG: &str = +pub(crate) const BOOL_AFTER_BACKGROUND_ERROR_MSG: &str = "The '%ls' command can not be used immediately after a backgrounded job"; /// Error message for backgrounded commands as conditionals. diff --git a/src/tests/parse_util.rs b/src/tests/parse_util.rs index ff1a940c2..24d744aa1 100644 --- a/src/tests/parse_util.rs +++ b/src/tests/parse_util.rs @@ -5,7 +5,7 @@ use crate::parse_constants::{ ERROR_NOT_ARGV_AT, ERROR_NOT_ARGV_COUNT, ERROR_NOT_ARGV_STAR, ERROR_NOT_PID, ERROR_NOT_STATUS, ERROR_NO_VAR_NAME, }; -use crate::parse_util::parse_util_detect_errors; +use crate::parse_util::{parse_util_detect_errors, BOOL_AFTER_BACKGROUND_ERROR_MSG}; use crate::tests::prelude::*; use crate::wchar::prelude::*; use crate::wchar_ext::WExt; @@ -22,10 +22,15 @@ fn test_error_messages() { let mut offset = 0; for mtch in format_specifier_regex.find_iter(format.as_char_slice()) { let mtch = mtch.unwrap(); - result.push(&format[offset..mtch.start()]); + let component = &format[offset..mtch.start()]; + result.push(component); offset = mtch.end(); } result.push(&format[offset..]); + // Avoid mismatch from localized quotes. + for component in &mut result { + *component = component.trim_matches('\''); + } result } @@ -74,4 +79,9 @@ fn test_error_messages() { validate!("echo foo\"$\"bar", ERROR_NO_VAR_NAME); validate!("echo \"foo\"$\"bar\"", ERROR_NO_VAR_NAME); validate!("echo foo $ bar", ERROR_NO_VAR_NAME); + validate!("echo 1 & && echo 2", BOOL_AFTER_BACKGROUND_ERROR_MSG); + validate!( + "echo 1 && echo 2 & && echo 3", + BOOL_AFTER_BACKGROUND_ERROR_MSG + ); } diff --git a/src/wchar_ext.rs b/src/wchar_ext.rs index 6f4139093..efa48e3e3 100644 --- a/src/wchar_ext.rs +++ b/src/wchar_ext.rs @@ -3,6 +3,7 @@ use std::{iter, slice}; use crate::{ common::subslice_position, wchar::{wstr, WString}, + L, }; use widestring::utfstr::CharsUtf32; @@ -284,6 +285,17 @@ pub trait WExt { self.as_char_slice().iter().copied().rev(), ) } + + fn trim_matches(&self, pat: char) -> &wstr { + let slice = self.as_char_slice(); + let leading_count = slice.chars().take_while(|&c| c == pat).count(); + let trailing_count = slice.chars().rev().take_while(|&c| c == pat).count(); + if leading_count == slice.len() { + return L!(""); + } + let slice = self.slice_from(leading_count); + slice.slice_to(slice.len() - trailing_count) + } } impl WExt for WString { @@ -396,4 +408,11 @@ mod tests { let needle = L!("hello world"); assert_eq!(haystack.find(needle), None); } + + #[test] + fn test_trim_matches() { + assert_eq!(L!("|foo|").trim_matches('|'), L!("foo")); + assert_eq!(L!("").trim_matches('|'), L!("foo>")); + } }