fix(api): reject empty ticker in resolve_symbol()

- Change resolve_symbol return type from String to Result<String, IdxError>
- Add early validation for empty/whitespace-only ticker input
- Return Err(IdxError::InvalidInput) for empty tickers
- Add InvalidInput variant to IdxError enum
- Update all 15 callers in cli/stocks.rs to handle Result with ? propagation
- Update tests: empty/whitespace tickers now return error

Fixes #7
This commit is contained in:
Ciphercat 2026-03-12 16:48:30 +00:00
commit cd4d8d8323
3 changed files with 37 additions and 23 deletions

View file

@ -62,15 +62,20 @@ pub trait NewsProvider {
fn news(&self, symbol: &str, limit: usize) -> Result<Vec<NewsItem>, IdxError>; fn news(&self, symbol: &str, limit: usize) -> Result<Vec<NewsItem>, IdxError>;
} }
pub fn resolve_symbol(symbol: &str, exchange: &str) -> String { pub fn resolve_symbol(symbol: &str, exchange: &str) -> Result<String, IdxError> {
let trimmed = symbol.trim().to_uppercase(); let trimmed = symbol.trim().to_uppercase();
if trimmed.is_empty() {
return Err(IdxError::InvalidInput(
"ticker symbol cannot be empty".into(),
));
}
if let Some((base, suffix)) = trimmed.rsplit_once('.') if let Some((base, suffix)) = trimmed.rsplit_once('.')
&& !base.is_empty() && !base.is_empty()
&& !suffix.is_empty() && !suffix.is_empty()
{ {
return trimmed; return Ok(trimmed);
} }
format!("{trimmed}.{}", exchange.trim().to_uppercase()) Ok(format!("{trimmed}.{}", exchange.trim().to_uppercase()))
} }
pub fn default_provider(provider: ProviderKind, verbose: bool) -> Box<dyn MarketDataProvider> { pub fn default_provider(provider: ProviderKind, verbose: bool) -> Box<dyn MarketDataProvider> {
@ -224,11 +229,16 @@ mod tests {
#[test] #[test]
fn resolves_symbol_variants() { fn resolves_symbol_variants() {
assert_eq!(resolve_symbol("bbca", "JK"), "BBCA.JK"); assert_eq!(resolve_symbol("bbca", "JK").unwrap(), "BBCA.JK");
assert_eq!(resolve_symbol("BBCA.JK", "JK"), "BBCA.JK"); assert_eq!(resolve_symbol("BBCA.JK", "JK").unwrap(), "BBCA.JK");
assert_eq!(resolve_symbol("TLKM.us", "JK"), "TLKM.US"); assert_eq!(resolve_symbol("TLKM.us", "JK").unwrap(), "TLKM.US");
assert_eq!(resolve_symbol("abcd.ef.gh", "JK"), "ABCD.EF.GH"); assert_eq!(resolve_symbol("abcd.ef.gh", "JK").unwrap(), "ABCD.EF.GH");
assert_eq!(resolve_symbol(" bbri ", "jk"), "BBRI.JK"); assert_eq!(resolve_symbol(" bbri ", "jk").unwrap(), "BBRI.JK");
assert_eq!(resolve_symbol("", "JK"), ".JK"); // Empty ticker should return error
assert!(resolve_symbol("", "JK").is_err());
// Whitespace-only ticker should also return error
assert!(resolve_symbol(" ", "JK").is_err());
// Valid ticker returns Ok
assert_eq!(resolve_symbol("BBCA", "JK").unwrap(), "BBCA.JK");
} }
} }

View file

@ -171,7 +171,7 @@ pub fn handle(
let quote_bucket = cache_bucket(config, "quote"); let quote_bucket = cache_bucket(config, "quote");
let mut quotes = Vec::new(); let mut quotes = Vec::new();
for sym in symbols.iter().flat_map(|s| s.split(',')) { for sym in symbols.iter().flat_map(|s| s.split(',')) {
let resolved = crate::api::resolve_symbol(sym, &config.exchange); let resolved = crate::api::resolve_symbol(sym, &config.exchange)?;
if !no_cache && let Some(q) = cache.get(&quote_bucket, &resolved)? { if !no_cache && let Some(q) = cache.get(&quote_bucket, &resolved)? {
quotes.push(q); quotes.push(q);
continue; continue;
@ -227,7 +227,7 @@ pub fn handle(
); );
} }
let history_bucket = format!("{}-history", history_source.as_str()); let history_bucket = format!("{}-history", history_source.as_str());
let resolved = crate::api::resolve_symbol(symbol, &config.exchange); let resolved = crate::api::resolve_symbol(symbol, &config.exchange)?;
let key = format!("{}-{}", period.as_str(), interval.as_str()); let key = format!("{}-{}", period.as_str(), interval.as_str());
if !no_cache if !no_cache
&& let Some(history) = cache.get::<Vec<crate::api::types::Ohlc>>( && let Some(history) = cache.get::<Vec<crate::api::types::Ohlc>>(
@ -293,7 +293,7 @@ pub fn handle(
); );
} }
let technical_bucket = format!("{}-technical", history_source.as_str()); let technical_bucket = format!("{}-technical", history_source.as_str());
let resolved = crate::api::resolve_symbol(symbol, &config.exchange); let resolved = crate::api::resolve_symbol(symbol, &config.exchange)?;
if !no_cache if !no_cache
&& let Some(report) = cache.get::<TechnicalReport>(&technical_bucket, &resolved)? && let Some(report) = cache.get::<TechnicalReport>(&technical_bucket, &resolved)?
{ {
@ -327,7 +327,7 @@ pub fn handle(
} }
} }
StocksSubcommand::Growth { symbol } => { StocksSubcommand::Growth { symbol } => {
let resolved = crate::api::resolve_symbol(symbol, &config.exchange); let resolved = crate::api::resolve_symbol(symbol, &config.exchange)?;
let report: GrowthReport = fetch_fundamental_analysis_report( let report: GrowthReport = fetch_fundamental_analysis_report(
&cache, &cache,
provider, provider,
@ -343,7 +343,7 @@ pub fn handle(
render_growth(&resolved, &report, &config.output, config.no_color) render_growth(&resolved, &report, &config.output, config.no_color)
} }
StocksSubcommand::Valuation { symbol } => { StocksSubcommand::Valuation { symbol } => {
let resolved = crate::api::resolve_symbol(symbol, &config.exchange); let resolved = crate::api::resolve_symbol(symbol, &config.exchange)?;
let report: ValuationReport = fetch_fundamental_analysis_report( let report: ValuationReport = fetch_fundamental_analysis_report(
&cache, &cache,
provider, provider,
@ -359,7 +359,7 @@ pub fn handle(
render_valuation(&resolved, &report, &config.output, config.no_color) render_valuation(&resolved, &report, &config.output, config.no_color)
} }
StocksSubcommand::Risk { symbol } => { StocksSubcommand::Risk { symbol } => {
let resolved = crate::api::resolve_symbol(symbol, &config.exchange); let resolved = crate::api::resolve_symbol(symbol, &config.exchange)?;
let report: RiskReport = fetch_fundamental_analysis_report( let report: RiskReport = fetch_fundamental_analysis_report(
&cache, &cache,
provider, provider,
@ -375,7 +375,7 @@ pub fn handle(
render_risk(&resolved, &report, &config.output, config.no_color) render_risk(&resolved, &report, &config.output, config.no_color)
} }
StocksSubcommand::Fundamental { symbol } => { StocksSubcommand::Fundamental { symbol } => {
let resolved = crate::api::resolve_symbol(symbol, &config.exchange); let resolved = crate::api::resolve_symbol(symbol, &config.exchange)?;
let report: FundamentalReport = fetch_fundamental_analysis_report( let report: FundamentalReport = fetch_fundamental_analysis_report(
&cache, &cache,
provider, provider,
@ -391,7 +391,7 @@ pub fn handle(
render_fundamental(&report, &config.output, config.no_color) render_fundamental(&report, &config.output, config.no_color)
} }
StocksSubcommand::Profile { symbol } => { StocksSubcommand::Profile { symbol } => {
let resolved = crate::api::resolve_symbol(symbol, &config.exchange); let resolved = crate::api::resolve_symbol(symbol, &config.exchange)?;
let profile: CompanyProfile = fetch_msn_only(&resolved, config.provider, || { let profile: CompanyProfile = fetch_msn_only(&resolved, config.provider, || {
MsnProvider::new(false).profile(&resolved) MsnProvider::new(false).profile(&resolved)
})?; })?;
@ -401,7 +401,7 @@ pub fn handle(
symbol, symbol,
statement: _, statement: _,
} => { } => {
let resolved = crate::api::resolve_symbol(symbol, &config.exchange); let resolved = crate::api::resolve_symbol(symbol, &config.exchange)?;
let financials: FinancialStatements = let financials: FinancialStatements =
fetch_msn_only(&resolved, config.provider, || { fetch_msn_only(&resolved, config.provider, || {
MsnProvider::new(false).financials(&resolved) MsnProvider::new(false).financials(&resolved)
@ -415,28 +415,28 @@ pub fn handle(
forecast: _, forecast: _,
history: _, history: _,
} => { } => {
let resolved = crate::api::resolve_symbol(symbol, &config.exchange); let resolved = crate::api::resolve_symbol(symbol, &config.exchange)?;
let earnings: EarningsReport = fetch_msn_only(&resolved, config.provider, || { let earnings: EarningsReport = fetch_msn_only(&resolved, config.provider, || {
MsnProvider::new(false).earnings(&resolved) MsnProvider::new(false).earnings(&resolved)
})?; })?;
render_earnings(&earnings, &config.output) render_earnings(&earnings, &config.output)
} }
StocksSubcommand::Sentiment { symbol } => { StocksSubcommand::Sentiment { symbol } => {
let resolved = crate::api::resolve_symbol(symbol, &config.exchange); let resolved = crate::api::resolve_symbol(symbol, &config.exchange)?;
let sentiment: SentimentData = fetch_msn_only(&resolved, config.provider, || { let sentiment: SentimentData = fetch_msn_only(&resolved, config.provider, || {
MsnProvider::new(false).sentiment(&resolved) MsnProvider::new(false).sentiment(&resolved)
})?; })?;
render_sentiment(&sentiment, &config.output) render_sentiment(&sentiment, &config.output)
} }
StocksSubcommand::Insights { symbol } => { StocksSubcommand::Insights { symbol } => {
let resolved = crate::api::resolve_symbol(symbol, &config.exchange); let resolved = crate::api::resolve_symbol(symbol, &config.exchange)?;
let insights: InsightData = fetch_msn_only(&resolved, config.provider, || { let insights: InsightData = fetch_msn_only(&resolved, config.provider, || {
MsnProvider::new(false).insights(&resolved) MsnProvider::new(false).insights(&resolved)
})?; })?;
render_insights(&insights, &config.output) render_insights(&insights, &config.output)
} }
StocksSubcommand::News { symbol, limit } => { StocksSubcommand::News { symbol, limit } => {
let resolved = crate::api::resolve_symbol(symbol, &config.exchange); let resolved = crate::api::resolve_symbol(symbol, &config.exchange)?;
let news: Vec<NewsItem> = fetch_msn_only(&resolved, config.provider, || { let news: Vec<NewsItem> = fetch_msn_only(&resolved, config.provider, || {
MsnProvider::new(false).news(&resolved, *limit) MsnProvider::new(false).news(&resolved, *limit)
})?; })?;
@ -460,7 +460,7 @@ pub fn handle(
let mut last_error = None; let mut last_error = None;
for sym in symbols.iter().flat_map(|s| s.split(',')) { for sym in symbols.iter().flat_map(|s| s.split(',')) {
let resolved = crate::api::resolve_symbol(sym, &config.exchange); let resolved = crate::api::resolve_symbol(sym, &config.exchange)?;
match fetch_fundamental_analysis_report( match fetch_fundamental_analysis_report(
&cache, &cache,
provider, provider,

View file

@ -30,6 +30,8 @@ pub enum IdxError {
DatabaseError(String), DatabaseError(String),
#[error("PDF parse error: {0}")] #[error("PDF parse error: {0}")]
PdfParseError(String), PdfParseError(String),
#[error("invalid input: {0}")]
InvalidInput(String),
} }
#[derive(Debug, Clone, Copy, Serialize, PartialEq, Eq)] #[derive(Debug, Clone, Copy, Serialize, PartialEq, Eq)]
@ -47,6 +49,7 @@ pub enum ErrorCode {
AuthError, AuthError,
DatabaseError, DatabaseError,
PdfParseError, PdfParseError,
InvalidInput,
} }
impl IdxError { impl IdxError {
@ -65,6 +68,7 @@ impl IdxError {
Self::AuthError(_) => ErrorCode::AuthError, Self::AuthError(_) => ErrorCode::AuthError,
Self::DatabaseError(_) => ErrorCode::DatabaseError, Self::DatabaseError(_) => ErrorCode::DatabaseError,
Self::PdfParseError(_) => ErrorCode::PdfParseError, Self::PdfParseError(_) => ErrorCode::PdfParseError,
Self::InvalidInput(_) => ErrorCode::InvalidInput,
} }
} }