Make json require string and pass around metadata (#7010)

* Make json require string and pass around metadata

The json deserializer was accepting any inputs by coercing non-strings
into strings. As an example, if the input was `[1, 2]` the coercion
would turn into `[12]` and deserialize as a list containing number
twelve instead of a list of two numbers, one and two. This could lead
to silent data corruption.

Aside from that pipeline metadata wasn't passed aroud.

This commit fixes the type issue by adding a strict conversion
function that errors if the input type is not a string or external
stream. It then uses this function instead of the original
`collect_string()`. In addition, this function returns the pipeline
metadata so it can be passed along.

* Make other formats require string

The problem with json coercing non-string types to string was present in
all other text formats. This reuses the `collect_string_strict` function
to fix them.

* `IntoPipelineData` cleanup

The method `into_pipeline_data_with_metadata` can now be conveniently
used.
This commit is contained in:
Martin Habovštiak 2022-11-21 02:06:09 +01:00 committed by GitHub
parent d01ccd5a54
commit d9d6cea5a9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 104 additions and 95 deletions

View File

@ -116,7 +116,6 @@ fn from_csv(
let noheaders = call.has_flag("noheaders");
let separator: Option<Value> = call.get_flag(engine_state, stack, "separator")?;
let trim: Option<Value> = call.get_flag(engine_state, stack, "trim")?;
let config = engine_state.get_config();
let sep = match separator {
Some(Value::String { val: s, span }) => {
@ -138,7 +137,7 @@ fn from_csv(
let trim = trim_from_str(trim)?;
from_delimited_data(noheaders, no_infer, sep, trim, input, name, config)
from_delimited_data(noheaders, no_infer, sep, trim, input, name)
}
#[cfg(test)]

View File

@ -1,5 +1,5 @@
use csv::{ReaderBuilder, Trim};
use nu_protocol::{Config, IntoPipelineData, PipelineData, ShellError, Span, Value};
use nu_protocol::{IntoPipelineData, PipelineData, ShellError, Span, Value};
fn from_delimited_string_to_value(
s: String,
@ -63,14 +63,13 @@ pub fn from_delimited_data(
trim: Trim,
input: PipelineData,
name: Span,
config: &Config,
) -> Result<PipelineData, ShellError> {
let concat_string = input.collect_string("", config)?;
let (concat_string, metadata) = input.collect_string_strict(name)?;
Ok(
from_delimited_string_to_value(concat_string, noheaders, no_infer, sep, trim, name)
.map_err(|x| ShellError::DelimiterError(x.to_string(), name))?
.into_pipeline_data(),
.into_pipeline_data_with_metadata(metadata),
)
}

View File

@ -5,7 +5,6 @@ use nu_engine::CallExt;
use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::Category;
use nu_protocol::Config;
use nu_protocol::{
Example, PipelineData, ShellError, Signature, Span, Spanned, SyntaxShape, Type, Value,
};
@ -46,8 +45,7 @@ impl Command for FromEml {
let head = call.head;
let preview_body: Option<Spanned<i64>> =
call.get_flag(engine_state, stack, "preview-body")?;
let config = engine_state.get_config();
from_eml(input, preview_body, head, config)
from_eml(input, preview_body, head)
}
fn examples(&self) -> Vec<Example> {
@ -182,9 +180,8 @@ fn from_eml(
input: PipelineData,
preview_body: Option<Spanned<i64>>,
head: Span,
config: &Config,
) -> Result<PipelineData, ShellError> {
let value = input.collect_string("", config)?;
let (value, metadata) = input.collect_string_strict(head)?;
let body_preview = preview_body
.map(|b| b.item as usize)
@ -236,7 +233,7 @@ fn from_eml(
item: collected,
span: head,
}),
None,
metadata,
))
}

View File

@ -5,8 +5,8 @@ use indexmap::map::IndexMap;
use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::{
Category, Config, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span,
Spanned, Type, Value,
Category, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, Spanned, Type,
Value,
};
use std::io::BufReader;
@ -30,14 +30,13 @@ impl Command for FromIcs {
fn run(
&self,
engine_state: &EngineState,
_engine_state: &EngineState,
_stack: &mut Stack,
call: &Call,
input: PipelineData,
) -> Result<nu_protocol::PipelineData, ShellError> {
let head = call.head;
let config = engine_state.get_config();
from_ics(input, head, config)
from_ics(input, head)
}
fn examples(&self) -> Vec<Example> {
@ -94,8 +93,8 @@ END:VCALENDAR' | from ics",
}
}
fn from_ics(input: PipelineData, head: Span, config: &Config) -> Result<PipelineData, ShellError> {
let input_string = input.collect_string("", config)?;
fn from_ics(input: PipelineData, head: Span) -> Result<PipelineData, ShellError> {
let (input_string, metadata) = input.collect_string_strict(head)?;
let input_string = input_string
.lines()
@ -124,7 +123,7 @@ fn from_ics(input: PipelineData, head: Span, config: &Config) -> Result<Pipeline
vals: output,
span: head,
}
.into_pipeline_data())
.into_pipeline_data_with_metadata(metadata))
}
fn calendar_to_value(calendar: IcalCalendar, span: Span) -> Value {

View File

@ -2,8 +2,7 @@ use indexmap::map::IndexMap;
use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::{
Category, Config, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, Type,
Value,
Category, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, Type, Value,
};
#[derive(Clone)]
@ -53,14 +52,13 @@ b=2' | from ini",
fn run(
&self,
engine_state: &EngineState,
_engine_state: &EngineState,
_stack: &mut Stack,
call: &Call,
input: PipelineData,
) -> Result<nu_protocol::PipelineData, ShellError> {
let head = call.head;
let config = engine_state.get_config();
from_ini(input, head, config)
from_ini(input, head)
}
}
@ -90,11 +88,11 @@ pub fn from_ini_string_to_value(s: String, span: Span) -> Result<Value, ShellErr
}
}
fn from_ini(input: PipelineData, head: Span, config: &Config) -> Result<PipelineData, ShellError> {
let concat_string = input.collect_string("", config)?;
fn from_ini(input: PipelineData, head: Span) -> Result<PipelineData, ShellError> {
let (concat_string, metadata) = input.collect_string_strict(head)?;
match from_ini_string_to_value(concat_string, head) {
Ok(x) => Ok(x.into_pipeline_data()),
Ok(x) => Ok(x.into_pipeline_data_with_metadata(metadata)),
Err(other) => Err(other),
}
}

View File

@ -76,11 +76,10 @@ impl Command for FromJson {
input: PipelineData,
) -> Result<nu_protocol::PipelineData, ShellError> {
let span = call.head;
let config = engine_state.get_config();
let string_input = input.collect_string("", config)?;
let (string_input, metadata) = input.collect_string_strict(span)?;
if string_input.is_empty() {
return Ok(PipelineData::new(span));
return Ok(PipelineData::new_with_metadata(metadata, span));
}
// TODO: turn this into a structured underline of the nu_json error
@ -98,9 +97,11 @@ impl Command for FromJson {
}
})
.collect();
Ok(converted_lines.into_pipeline_data(engine_state.ctrlc.clone()))
Ok(converted_lines
.into_pipeline_data_with_metadata(metadata, engine_state.ctrlc.clone()))
} else {
Ok(convert_string_to_value(string_input, span)?.into_pipeline_data())
Ok(convert_string_to_value(string_input, span)?
.into_pipeline_data_with_metadata(metadata))
}
}
}

View File

@ -68,14 +68,13 @@ impl Command for FromNuon {
fn run(
&self,
engine_state: &EngineState,
_engine_state: &EngineState,
_stack: &mut Stack,
call: &Call,
input: PipelineData,
) -> Result<nu_protocol::PipelineData, ShellError> {
let head = call.head;
let config = engine_state.get_config();
let string_input = input.collect_string("", config)?;
let (string_input, metadata) = input.collect_string_strict(head)?;
let engine_state = EngineState::new();
@ -183,7 +182,7 @@ impl Command for FromNuon {
let result = convert_to_value(expr, head, &string_input);
match result {
Ok(result) => Ok(result.into_pipeline_data()),
Ok(result) => Ok(result.into_pipeline_data_with_metadata(metadata)),
Err(err) => Err(ShellError::GenericError(
"error when loading nuon text".into(),
"could not load nuon text".into(),

View File

@ -268,7 +268,6 @@ fn from_ssv(
call: &Call,
input: PipelineData,
) -> Result<PipelineData, ShellError> {
let config = engine_state.get_config();
let name = call.head;
let noheaders = call.has_flag("noheaders");
@ -276,7 +275,7 @@ fn from_ssv(
let minimum_spaces: Option<Spanned<usize>> =
call.get_flag(engine_state, stack, "minimum-spaces")?;
let concat_string = input.collect_string("", config)?;
let (concat_string, metadata) = input.collect_string_strict(name)?;
let split_at = match minimum_spaces {
Some(number) => number.item,
None => DEFAULT_MINIMUM_SPACES,
@ -284,7 +283,7 @@ fn from_ssv(
Ok(
from_ssv_string_to_value(&concat_string, noheaders, aligned_columns, split_at, name)
.into_pipeline_data(),
.into_pipeline_data_with_metadata(metadata),
)
}

View File

@ -69,16 +69,15 @@ b = [1, 2]' | from toml",
fn run(
&self,
engine_state: &EngineState,
__engine_state: &EngineState,
_stack: &mut Stack,
call: &Call,
input: PipelineData,
) -> Result<nu_protocol::PipelineData, ShellError> {
let span = call.head;
let config = engine_state.get_config();
let mut string_input = input.collect_string("", config)?;
let (mut string_input, metadata) = input.collect_string_strict(span)?;
string_input.push('\n');
Ok(convert_string_to_value(string_input, span)?.into_pipeline_data())
Ok(convert_string_to_value(string_input, span)?.into_pipeline_data_with_metadata(metadata))
}
}

View File

@ -106,15 +106,7 @@ fn from_tsv(
let trim: Option<Value> = call.get_flag(engine_state, stack, "trim")?;
let trim = trim_from_str(trim)?;
from_delimited_data(
noheaders,
no_infer,
'\t',
trim,
input,
name,
engine_state.get_config(),
)
from_delimited_data(noheaders, no_infer, '\t', trim, input, name)
}
#[cfg(test)]

View File

@ -1,8 +1,6 @@
use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::{
Category, Config, Example, PipelineData, ShellError, Signature, Span, Type, Value,
};
use nu_protocol::{Category, Example, PipelineData, ShellError, Signature, Span, Type, Value};
#[derive(Clone)]
pub struct FromUrl;
@ -24,14 +22,13 @@ impl Command for FromUrl {
fn run(
&self,
engine_state: &EngineState,
_engine_state: &EngineState,
_stack: &mut Stack,
call: &Call,
input: PipelineData,
) -> Result<nu_protocol::PipelineData, ShellError> {
let head = call.head;
let config = engine_state.get_config();
from_url(input, head, config)
from_url(input, head)
}
fn examples(&self) -> Vec<Example> {
@ -57,8 +54,8 @@ impl Command for FromUrl {
}
}
fn from_url(input: PipelineData, head: Span, config: &Config) -> Result<PipelineData, ShellError> {
let concat_string = input.collect_string("", config)?;
fn from_url(input: PipelineData, head: Span) -> Result<PipelineData, ShellError> {
let (concat_string, metadata) = input.collect_string_strict(head)?;
let result = serde_urlencoded::from_str::<Vec<(String, String)>>(&concat_string);
@ -77,7 +74,7 @@ fn from_url(input: PipelineData, head: Span, config: &Config) -> Result<Pipeline
vals,
span: head,
},
None,
metadata,
))
}
_ => Err(ShellError::UnsupportedInput(

View File

@ -4,8 +4,8 @@ use indexmap::map::IndexMap;
use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::{
Category, Config, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span,
Spanned, Type, Value,
Category, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, Spanned, Type,
Value,
};
#[derive(Clone)]
@ -28,14 +28,13 @@ impl Command for FromVcf {
fn run(
&self,
engine_state: &EngineState,
_engine_state: &EngineState,
_stack: &mut Stack,
call: &Call,
input: PipelineData,
) -> Result<nu_protocol::PipelineData, ShellError> {
let head = call.head;
let config = engine_state.get_config();
from_vcf(input, head, config)
from_vcf(input, head)
}
fn examples(&self) -> Vec<Example> {
@ -125,8 +124,8 @@ END:VCARD' | from vcf",
}
}
fn from_vcf(input: PipelineData, head: Span, config: &Config) -> Result<PipelineData, ShellError> {
let input_string = input.collect_string("", config)?;
fn from_vcf(input: PipelineData, head: Span) -> Result<PipelineData, ShellError> {
let (input_string, metadata) = input.collect_string_strict(head)?;
let input_string = input_string
.lines()
@ -153,7 +152,7 @@ fn from_vcf(input: PipelineData, head: Span, config: &Config) -> Result<Pipeline
vals: collected,
span: head,
}
.into_pipeline_data())
.into_pipeline_data_with_metadata(metadata))
}
fn contact_to_value(contact: VcardContact, span: Span) -> Value {

View File

@ -2,8 +2,8 @@ use indexmap::map::IndexMap;
use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::{
Category, Config, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span,
Spanned, Type, Value,
Category, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, Spanned, Type,
Value,
};
#[derive(Clone)]
@ -26,14 +26,13 @@ impl Command for FromXml {
fn run(
&self,
engine_state: &EngineState,
_engine_state: &EngineState,
_stack: &mut Stack,
call: &Call,
input: PipelineData,
) -> Result<nu_protocol::PipelineData, ShellError> {
let head = call.head;
let config = engine_state.get_config();
from_xml(input, head, config)
from_xml(input, head)
}
fn examples(&self) -> Vec<Example> {
@ -180,11 +179,11 @@ pub fn from_xml_string_to_value(s: String, span: Span) -> Result<Value, roxmltre
Ok(from_document_to_value(&parsed, span))
}
fn from_xml(input: PipelineData, head: Span, config: &Config) -> Result<PipelineData, ShellError> {
let concat_string = input.collect_string("", config)?;
fn from_xml(input: PipelineData, head: Span) -> Result<PipelineData, ShellError> {
let (concat_string, metadata) = input.collect_string_strict(head)?;
match from_xml_string_to_value(concat_string, head) {
Ok(x) => Ok(x.into_pipeline_data()),
Ok(x) => Ok(x.into_pipeline_data_with_metadata(metadata)),
_ => Err(ShellError::UnsupportedInput(
"Could not parse string as xml".to_string(),
head,

View File

@ -2,8 +2,8 @@ use itertools::Itertools;
use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::{
Category, Config, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span,
Spanned, Type, Value,
Category, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, Spanned, Type,
Value,
};
use serde::de::Deserialize;
use std::collections::HashMap;
@ -32,14 +32,13 @@ impl Command for FromYaml {
fn run(
&self,
engine_state: &EngineState,
_engine_state: &EngineState,
_stack: &mut Stack,
call: &Call,
input: PipelineData,
) -> Result<nu_protocol::PipelineData, ShellError> {
let head = call.head;
let config = engine_state.get_config();
from_yaml(input, head, config)
from_yaml(input, head)
}
}
@ -61,14 +60,13 @@ impl Command for FromYml {
fn run(
&self,
engine_state: &EngineState,
_engine_state: &EngineState,
_stack: &mut Stack,
call: &Call,
input: PipelineData,
) -> Result<nu_protocol::PipelineData, ShellError> {
let head = call.head;
let config = engine_state.get_config();
from_yaml(input, head, config)
from_yaml(input, head)
}
fn examples(&self) -> Vec<Example> {
@ -215,11 +213,11 @@ pub fn get_examples() -> Vec<Example> {
]
}
fn from_yaml(input: PipelineData, head: Span, config: &Config) -> Result<PipelineData, ShellError> {
let concat_string = input.collect_string("", config)?;
fn from_yaml(input: PipelineData, head: Span) -> Result<PipelineData, ShellError> {
let (concat_string, metadata) = input.collect_string_strict(head)?;
match from_yaml_string_to_value(concat_string, head) {
Ok(x) => Ok(x.into_pipeline_data()),
Ok(x) => Ok(x.into_pipeline_data_with_metadata(metadata)),
Err(other) => Err(other),
}
}
@ -227,6 +225,7 @@ fn from_yaml(input: PipelineData, head: Span, config: &Config) -> Result<Pipelin
#[cfg(test)]
mod test {
use super::*;
use nu_protocol::Config;
#[test]
fn test_problematic_yaml() {

View File

@ -211,6 +211,33 @@ impl PipelineData {
}
}
/// Retrieves string from pipline data.
///
/// As opposed to `collect_string` this raises error rather than converting non-string values.
/// The `span` will be used if `ListStream` is encountered since it doesn't carry a span.
pub fn collect_string_strict(
self,
span: Span,
) -> Result<(String, Option<PipelineMetadata>), ShellError> {
match self {
PipelineData::Value(Value::String { val, .. }, metadata) => Ok((val, metadata)),
PipelineData::Value(val, _) => {
Err(ShellError::TypeMismatch("string".into(), val.span()?))
}
PipelineData::ListStream(_, _) => Err(ShellError::TypeMismatch("string".into(), span)),
PipelineData::ExternalStream {
stdout: None,
metadata,
..
} => Ok((String::new(), metadata)),
PipelineData::ExternalStream {
stdout: Some(stdout),
metadata,
..
} => Ok((stdout.into_string()?.item, metadata)),
}
}
pub fn follow_cell_path(
self,
cell_path: &[PathMember],
@ -596,7 +623,10 @@ impl Iterator for PipelineIterator {
pub trait IntoPipelineData {
fn into_pipeline_data(self) -> PipelineData;
fn into_pipeline_data_with_metadata(self, metadata: PipelineMetadata) -> PipelineData;
fn into_pipeline_data_with_metadata(
self,
metadata: impl Into<Option<PipelineMetadata>>,
) -> PipelineData;
}
impl<V> IntoPipelineData for V
@ -606,8 +636,11 @@ where
fn into_pipeline_data(self) -> PipelineData {
PipelineData::Value(self.into(), None)
}
fn into_pipeline_data_with_metadata(self, metadata: PipelineMetadata) -> PipelineData {
PipelineData::Value(self.into(), Some(metadata))
fn into_pipeline_data_with_metadata(
self,
metadata: impl Into<Option<PipelineMetadata>>,
) -> PipelineData {
PipelineData::Value(self.into(), metadata.into())
}
}
@ -615,7 +648,7 @@ pub trait IntoInterruptiblePipelineData {
fn into_pipeline_data(self, ctrlc: Option<Arc<AtomicBool>>) -> PipelineData;
fn into_pipeline_data_with_metadata(
self,
metadata: PipelineMetadata,
metadata: impl Into<Option<PipelineMetadata>>,
ctrlc: Option<Arc<AtomicBool>>,
) -> PipelineData;
}
@ -638,7 +671,7 @@ where
fn into_pipeline_data_with_metadata(
self,
metadata: PipelineMetadata,
metadata: impl Into<Option<PipelineMetadata>>,
ctrlc: Option<Arc<AtomicBool>>,
) -> PipelineData {
PipelineData::ListStream(
@ -646,7 +679,7 @@ where
stream: Box::new(self.into_iter().map(Into::into)),
ctrlc,
},
Some(metadata),
metadata.into(),
)
}
}