Skip to content

Parser is done - #9

Closed
Basil-Ismail wants to merge 22 commits into
mainfrom
build
Closed

Basil-Ismail wants to merge 22 commits into
mainfrom
build

Conversation

@Basil-Ismail

Copy link
Copy Markdown
Member

@Basil-Ismail Basil-Ismail self-assigned this Sep 27, 2025
@Basil-Ismail Basil-Ismail added the enhancement New feature or request label Sep 27, 2025

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR completes the implementation of a configuration parser for a web server project, replacing incomplete parsing logic with a fully functional lexer and parser system. The implementation transitions from a basic file validation approach to a comprehensive token-based parsing system that can handle complex HTTP server configurations.

  • Implements a complete lexer that tokenizes configuration files with support for comments, quoted strings, and various token types
  • Adds a recursive descent parser that builds structured configuration objects from tokens
  • Introduces new model classes (LocationConfig, Container) to represent parsed configuration data

Reviewed Changes

Copilot reviewed 40 out of 44 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/models/srcs/parser.cpp Implements the main parser logic with functions for parsing server blocks, locations, and directives
src/models/srcs/lexer.cpp Implements tokenization of configuration files with support for comments and quoted strings
src/models/srcs/readFile.cpp Simple file reading utility function
src/models/srcs/LocationConfig.cpp New model class representing location blocks in configuration
src/models/srcs/Container.cpp New container class for holding multiple server configurations
src/models/headers/* Updated and new header files defining the parser interface and model classes
src/main.cpp Updated main function to use the new parser system and display parsed configuration
config/* New configuration test files for various parsing scenarios
Tests/* Comprehensive test suite for validating parser functionality
Makefile Updated build configuration to include new source files

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread src/models/srcs/lexer.cpp
bool isAttribute(const std::string &s)
{
return s == "root" || s == "client_max_body_size" || s == "listen" || s == "index" || s == "error_page" ||
s == "server_name" || s == "autoindex" || s == "redirect" || s == "index" || s == "cgi";

Copilot AI Sep 27, 2025

Copy link

Choose a reason for hiding this comment

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

The 'index' directive is listed twice in the condition, which is redundant and could lead to confusion during maintenance.

Suggested change
s == "server_name" || s == "autoindex" || s == "redirect" || s == "index" || s == "cgi";
s == "server_name" || s == "autoindex" || s == "redirect" || s == "cgi";

Copilot uses AI. Check for mistakes.
// Collect error codes
while (i < tokens.size() && tokens[i].value != ";" && tokens[i].type == NUMBER)
{
errorCodes.push_back(static_cast<u_int16_t>(std::atoi(tokens[i].value.c_str())));

Copilot AI Sep 27, 2025

Copy link

Choose a reason for hiding this comment

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

Using atoi() without validation could lead to undefined behavior if the token contains non-numeric characters. Consider using strtol() with proper error checking or validating that the token is numeric before conversion.

Copilot uses AI. Check for mistakes.
u_int16_t port = 80; // default
if (!tokens[i].value.empty())
{
port = static_cast<u_int16_t>(std::atoi(tokens[i].value.c_str()));

Copilot AI Sep 27, 2025

Copy link

Choose a reason for hiding this comment

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

Using atoi() without validation could result in undefined behavior for non-numeric input or values outside the valid port range (0-65535). Consider using strtol() with proper error checking.

Copilot uses AI. Check for mistakes.
Comment thread src/models/srcs/lexer.cpp
for (size_t i = 0; i < val.size(); i++)
{
char c = val[i];
if (!isalnum(c) && c != '_' && c != '.' && c != '/' && c != '-' && c != '=' && it->quoted == 0)

Copilot AI Sep 27, 2025

Copy link

Choose a reason for hiding this comment

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

The character validation logic is complex and hard to maintain. Consider extracting this into a separate function like isValidIdentifierChar() to improve readability and maintainability.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants