Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions cpp/src/io/mps_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,24 @@ ObjSenseType convert_to_obj_sense(const std::string& str)
}
}

RowType convert_to_row_type(char c)
{
switch (c) {
case 'N': return Objective;
case 'L': return LesserThanOrEqual;
case 'G': return GreaterThanOrEqual;
case 'E': return Equality;
default:
mps_parser_expects(
false,
error_type_t::ValidationError,
"Invalid row type '%c' (0x%02x) found in ROWS section; expected N, L, G, or E",
(c >= 0x20 && c < 0x7f) ? c : '?',
static_cast<unsigned char>(c));
return Objective; // unreachable; mps_parser_expects throws
}
}

template <typename i_t, typename f_t>
void mps_parser_t<i_t, f_t>::fill_problem(mps_data_model_t<i_t, f_t>& problem)
{
Expand Down Expand Up @@ -785,13 +803,13 @@ void mps_parser_t<i_t, f_t>::parse_rows(std::string_view line)
std::string name;

if (fixed_mps_format) {
type = static_cast<RowType>(line[1]);
type = convert_to_row_type(line[1]);
name = trim(line.substr(4, 8)); // max of 8 chars allowed
Comment on lines 805 to 807

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Guard fixed-format ROWS lines before indexing.

line[1] and line.substr(4, 8) run before any validation, so a short malformed ROWS record can still hit undefined behavior or throw std::out_of_range instead of the intended ValidationError.

Suggested fix
   if (fixed_mps_format) {
+    mps_parser_expects(line.size() >= 5,
+                       error_type_t::ValidationError,
+                       "ROWS should have at least a row type and name! line=%s",
+                       std::string(line).c_str());
     type = convert_to_row_type(line[1]);
     name = trim(line.substr(4, 8));  // max of 8 chars allowed
   } else {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (fixed_mps_format) {
type = static_cast<RowType>(line[1]);
type = convert_to_row_type(line[1]);
name = trim(line.substr(4, 8)); // max of 8 chars allowed
if (fixed_mps_format) {
mps_parser_expects(line.size() >= 5,
error_type_t::ValidationError,
"ROWS should have at least a row type and name! line=%s",
std::string(line).c_str());
type = convert_to_row_type(line[1]);
name = trim(line.substr(4, 8)); // max of 8 chars allowed
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/src/io/mps_parser.cpp` around lines 805 - 807, Guard the fixed-format
ROWS parsing in mps_parser.cpp before accessing line[1] or calling
line.substr(4, 8) inside the fixed_mps_format branch. Add a length validation in
the ROWS handling path used by convert_to_row_type and trim so malformed short
records fail with ValidationError first, instead of risking undefined behavior
or std::out_of_range.

} else {
std::stringstream ss{std::string(line)};
char read_word;
ss >> read_word;
type = static_cast<RowType>(read_word);
type = convert_to_row_type(read_word);
Comment on lines 809 to +812

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Parse the free-format row type as a token, not a single char.

operator>>(char) only consumes one non-whitespace byte. That means malformed input like NN COST is accepted as row type N, and if extraction fails read_word is used uninitialized. The new validation should reject the whole field, not just its first byte.

Suggested fix
   } else {
     std::stringstream ss{std::string(line)};
-    char read_word;
-    ss >> read_word;
-    type = convert_to_row_type(read_word);
+    std::string row_type_token;
+    ss >> row_type_token;
+    mps_parser_expects(!row_type_token.empty() && row_type_token.size() == 1,
+                       error_type_t::ValidationError,
+                       "Invalid row type '%s' found in ROWS section; expected N, L, G, or E",
+                       row_type_token.c_str());
+    type = convert_to_row_type(row_type_token[0]);
 
     ss >> name;
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
std::stringstream ss{std::string(line)};
char read_word;
ss >> read_word;
type = static_cast<RowType>(read_word);
type = convert_to_row_type(read_word);
std::stringstream ss{std::string(line)};
std::string row_type_token;
ss >> row_type_token;
mps_parser_expects(!row_type_token.empty() && row_type_token.size() == 1,
error_type_t::ValidationError,
"Invalid row type '%s' found in ROWS section; expected N, L, G, or E",
row_type_token.c_str());
type = convert_to_row_type(row_type_token[0]);
ss >> name;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/src/io/mps_parser.cpp` around lines 809 - 812, Parse the free-format row
type as a full token in the MPS parser instead of using a single char in the
row-type read path. In mps_parser.cpp, update the logic around std::stringstream
ss, read_word, and convert_to_row_type so it extracts the complete field,
validates that exactly one token was present, and rejects malformed inputs like
extra trailing text rather than accepting the first character only. Also ensure
the code does not use an uninitialized read_word when extraction fails.


ss >> name;
}
Expand Down