Merged
Conversation
Fixes #17 Implements a backward-compatible fix for the editcarinfo.php endpoint. The script now accepts either ID or �ssetID as the identifier for updates, prioritizing ID. This resolves the contract mismatch where the Flutter client was sending �ssetID while the backend expected the primary key ID. The endpoint now also uses prepared statements and returns a standardized JSON response.
Reviewer's GuideAligns the CarInfo edit backend endpoint with the expected API contract (ID or assetID, structured JSON, explicit HTTP status codes), tightens validation and error handling, updates the release workflow to Java 17, and introduces standard community and contribution documentation and GitHub templates. Sequence diagram for updated CarInfo edit endpoint behaviorsequenceDiagram
actor Client
participant EditCarInfoAPI as editcarinfo_php
participant DB as MySQL_carinfos
Client->>EditCarInfoAPI: POST /backend/carinfos/editcarinfo.php
activate EditCarInfoAPI
EditCarInfoAPI->>EditCarInfoAPI: Validate phpPassword
alt phpPassword invalid
EditCarInfoAPI-->>Client: 401 Unauthorized\n{ success: false, message: "Invalid PHP password." }
else phpPassword valid
EditCarInfoAPI->>EditCarInfoAPI: Read ID and assetID
alt missing ID and assetID
EditCarInfoAPI-->>Client: 400 Bad Request\n{ success: false, message: "A required identifier (ID or assetID) is missing." }
else identifier present
EditCarInfoAPI->>EditCarInfoAPI: Collect allowedFields from POST
alt no allowed fields provided
EditCarInfoAPI-->>Client: 500 Internal Server Error\n{ success: false, message: "No fields to update were provided." }
else fields to update
EditCarInfoAPI->>EditCarInfoAPI: Build UPDATE SQL with WHERE ID or assetID
EditCarInfoAPI->>DB: prepare(sql)
alt prepare fails
DB-->>EditCarInfoAPI: error
EditCarInfoAPI-->>Client: 500 Internal Server Error\n{ success: false, message: "SQL statement preparation failed: ..." }
else prepare ok
EditCarInfoAPI->>DB: bind_param(bindTypes, bindValues)
EditCarInfoAPI->>DB: execute()
alt execute fails
DB-->>EditCarInfoAPI: error
EditCarInfoAPI-->>Client: 500 Internal Server Error\n{ success: false, message: "SQL statement execution failed: ..." }
else execute ok
DB-->>EditCarInfoAPI: affected_rows
EditCarInfoAPI-->>Client: 200 OK\n{ success: true, message: "Car info updated successfully.", data: { affected_rows } }
end
end
end
end
end
deactivate EditCarInfoAPI
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- The new editcarinfo endpoint dropped the
seyruseferdateYYYY-MM-DDformat validation that existed before; if clients or downstream code rely on this constraint, consider reintroducing a simple format check to avoid silently accepting invalid dates. - For the
No fields to update were provided.case you currently return a 500 unless a code was set earlier; this appears to be a client error, so explicitly setting a 4xx status (e.g., 400) before throwing would make the API contract clearer and error handling more accurate.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new editcarinfo endpoint dropped the `seyruseferdate` `YYYY-MM-DD` format validation that existed before; if clients or downstream code rely on this constraint, consider reintroducing a simple format check to avoid silently accepting invalid dates.
- For the `No fields to update were provided.` case you currently return a 500 unless a code was set earlier; this appears to be a client error, so explicitly setting a 4xx status (e.g., 400) before throwing would make the API contract clearer and error handling more accurate.
## Individual Comments
### Comment 1
<location> `backend/carinfos/editcarinfo.php:36-41` </location>
<code_context>
+ throw new Exception('A required identifier (ID or assetID) is missing.');
+ }
+
+ $allowedFields = ['brand', 'sasino', 'engineno', 'enginepow', 'cartype', 'carusage', 'price', 'productionyear', 'seatcapacity', 'seyruseferdate'];
+ $setClauses = [];
+ $bindValues = [];
+ $bindTypes = '';
+
+ foreach ($allowedFields as $field) {
+ if (array_key_exists($field, $_POST)) {
+ $value = $_POST[$field];
</code_context>
<issue_to_address>
**issue (bug_risk):** The previous `seyruseferdate` format validation has been removed, which can allow invalid dates into the DB.
The new loop now treats `seyruseferdate` as a plain string and no longer enforces the `YYYY-MM-DD` format that was previously checked. If the DB or downstream logic assumes a valid date, this can introduce bad data and hard-to-track bugs. Please add a specific validation branch for `seyruseferdate` inside the loop to preserve the original guarantee.
</issue_to_address>
### Comment 2
<location> `backend/carinfos/editcarinfo.php:62-64` </location>
<code_context>
- $sql = "UPDATE carinfos SET ".implode(', ',$set)." WHERE ID = ?";
- $types.='i';
- $vals[] = $id;
+ if (empty($setClauses)) {
+ throw new Exception('No fields to update were provided.');
+ }
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Client-side input issues like “no fields to update” currently end up as HTTP 500 instead of a 4xx.
Because we throw here without setting an HTTP status, this case will be returned as 500. Since this is a client error, please set an appropriate 4xx status (e.g. `http_response_code(400);`) before throwing, consistent with how missing identifiers are handled, so clients can distinguish bad requests from real server errors.
```suggestion
if (empty($setClauses)) {
http_response_code(400);
throw new Exception('No fields to update were provided.');
}
```
</issue_to_address>
### Comment 3
<location> `backend/carinfos/editcarinfo.php:89-94` </location>
<code_context>
- $resp['success']=false;
- $resp['message']=$e->getMessage();
+
+} catch (Exception $e) {
+ if (http_response_code() === 200) { // If no HTTP code was set by us, it's an internal error
+ http_response_code(500);
+ }
+ $response['success'] = false;
+ $response['message'] = $e->getMessage();
}
-echo json_encode($resp,JSON_UNESCAPED_UNICODE);
</code_context>
<issue_to_address>
**🚨 issue (security):** Returning raw exception messages in the API response may leak internal details and SQL errors.
Some exceptions here include low-level messages like `SQL statement preparation failed: ...` or `SQL statement execution failed: ...`. Sending `$e->getMessage()` directly to clients can expose internal DB details. Instead, map exceptions to generic user-facing messages (e.g. "Internal server error") and log the detailed message server-side, only returning specific messages when they are known-safe (such as validation errors).
</issue_to_address>
### Comment 4
<location> `CODE_OF_CONDUCT.md:5` </location>
<code_context>
+
+## Our Pledge
+
+In the interest of fostering an open and welcoming environment, we as contributors and maintainers pledge to making participation in our project and our community a harassment-free experience for everyone, regardless of age, body size, disability, ethnicity, sex characteristics, gender identity and expression, level of experience, education, socio-economic status, nationality, personal appearance, race, religion, or sexual identity and orientation.
+
+## Our Standards
</code_context>
<issue_to_address>
**issue (typo):** Fix the phrasing "pledge to making" to the grammatically correct "pledge to make".
Change `pledge to making participation` to `pledge to make participation` for correct grammar and smoother reading.
```suggestion
In the interest of fostering an open and welcoming environment, we as contributors and maintainers pledge to make participation in our project and our community a harassment-free experience for everyone, regardless of age, body size, disability, ethnicity, sex characteristics, gender identity and expression, level of experience, education, socio-economic status, nationality, personal appearance, race, religion, or sexual identity and orientation.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+36
to
+41
| $allowedFields = ['brand', 'sasino', 'engineno', 'enginepow', 'cartype', 'carusage', 'price', 'productionyear', 'seatcapacity', 'seyruseferdate']; | ||
| $setClauses = []; | ||
| $bindValues = []; | ||
| $bindTypes = ''; | ||
|
|
||
| foreach ($allowedFields as $field) { |
There was a problem hiding this comment.
issue (bug_risk): The previous seyruseferdate format validation has been removed, which can allow invalid dates into the DB.
The new loop now treats seyruseferdate as a plain string and no longer enforces the YYYY-MM-DD format that was previously checked. If the DB or downstream logic assumes a valid date, this can introduce bad data and hard-to-track bugs. Please add a specific validation branch for seyruseferdate inside the loop to preserve the original guarantee.
Comment on lines
+62
to
+64
| if (empty($setClauses)) { | ||
| throw new Exception('No fields to update were provided.'); | ||
| } |
There was a problem hiding this comment.
suggestion (bug_risk): Client-side input issues like “no fields to update” currently end up as HTTP 500 instead of a 4xx.
Because we throw here without setting an HTTP status, this case will be returned as 500. Since this is a client error, please set an appropriate 4xx status (e.g. http_response_code(400);) before throwing, consistent with how missing identifiers are handled, so clients can distinguish bad requests from real server errors.
Suggested change
| if (empty($setClauses)) { | |
| throw new Exception('No fields to update were provided.'); | |
| } | |
| if (empty($setClauses)) { | |
| http_response_code(400); | |
| throw new Exception('No fields to update were provided.'); | |
| } |
Comment on lines
+89
to
+94
| } catch (Exception $e) { | ||
| if (http_response_code() === 200) { // If no HTTP code was set by us, it's an internal error | ||
| http_response_code(500); | ||
| } | ||
| $response['success'] = false; | ||
| $response['message'] = $e->getMessage(); |
There was a problem hiding this comment.
🚨 issue (security): Returning raw exception messages in the API response may leak internal details and SQL errors.
Some exceptions here include low-level messages like SQL statement preparation failed: ... or SQL statement execution failed: .... Sending $e->getMessage() directly to clients can expose internal DB details. Instead, map exceptions to generic user-facing messages (e.g. "Internal server error") and log the detailed message server-side, only returning specific messages when they are known-safe (such as validation errors).
|
|
||
| ## Our Pledge | ||
|
|
||
| In the interest of fostering an open and welcoming environment, we as contributors and maintainers pledge to making participation in our project and our community a harassment-free experience for everyone, regardless of age, body size, disability, ethnicity, sex characteristics, gender identity and expression, level of experience, education, socio-economic status, nationality, personal appearance, race, religion, or sexual identity and orientation. |
There was a problem hiding this comment.
issue (typo): Fix the phrasing "pledge to making" to the grammatically correct "pledge to make".
Change pledge to making participation to pledge to make participation for correct grammar and smoother reading.
Suggested change
| In the interest of fostering an open and welcoming environment, we as contributors and maintainers pledge to making participation in our project and our community a harassment-free experience for everyone, regardless of age, body size, disability, ethnicity, sex characteristics, gender identity and expression, level of experience, education, socio-economic status, nationality, personal appearance, race, religion, or sexual identity and orientation. | |
| In the interest of fostering an open and welcoming environment, we as contributors and maintainers pledge to make participation in our project and our community a harassment-free experience for everyone, regardless of age, body size, disability, ethnicity, sex characteristics, gender identity and expression, level of experience, education, socio-economic status, nationality, personal appearance, race, religion, or sexual identity and orientation. |
There was a problem hiding this comment.
Pull request overview
This PR addresses a high-priority API contract mismatch in the CarInfo edit endpoint and adds essential community documentation. The backend changes modernize the response structure and improve error handling, while the documentation additions establish project standards for contributors.
Key Changes:
- Refactored
editcarinfo.phpto support both ID and assetID for updates, with standardized response structure including adatawrapper - Added MIT License and community standards (Contributing guide, Code of Conduct)
- Updated Java version from 11 to 17 in GitHub Actions workflow
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/carinfos/editcarinfo.php | Refactored to support ID/assetID-based updates with improved error handling and standardized API response structure |
| LICENSE | Added MIT License with 2025 copyright for SigortaCu Contributors |
| CONTRIBUTING.md | Added contribution guidelines including code style standards (Flutter style guide, PSR-12 for PHP) |
| CODE_OF_CONDUCT.md | Added code of conduct defining community standards and behavior expectations |
| .github/workflows/release.yml | Updated Java version from 11 to 17 for compatibility with modern Flutter/Android builds |
| .github/PULL_REQUEST_TEMPLATE.md | Added PR template with standard sections for description, type of change, testing, and checklist |
| .github/ISSUE_TEMPLATE/feature_request.md | Added feature request template for structured enhancement proposals |
| .github/ISSUE_TEMPLATE/bug_report.md | Added bug report template for consistent issue reporting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+41
to
59
| foreach ($allowedFields as $field) { | ||
| if (array_key_exists($field, $_POST)) { | ||
| $value = $_POST[$field]; | ||
|
|
||
| // Basic validation and type setting | ||
| $isNumericField = in_array($field, ['enginepow', 'price', 'productionyear', 'seatcapacity']); | ||
| if ($isNumericField && $value !== '') { | ||
| $value = intval($value); | ||
| $bindTypes .= 'i'; | ||
| } else { | ||
| $types.='s'; | ||
| $bindTypes .= 's'; | ||
| } | ||
| if($val==='') $val=null; // boş ise null kaydet | ||
| $set[] = "$f = ?"; | ||
| $vals[] = $val; | ||
|
|
||
| // Set empty strings to null for the database | ||
| if ($value === '') $value = null; | ||
|
|
||
| $setClauses[] = "$field = ?"; | ||
| $bindValues[] = $value; | ||
| } |
There was a problem hiding this comment.
Missing date format validation for the 'seyruseferdate' field. The original code validated that this field matches the YYYY-MM-DD format when provided. This validation has been removed, which could allow invalid date formats to be inserted into the database.
Other carinfo endpoints (addcarinfo.php and addfullcarinfo.php) include this validation. For consistency and data integrity, you should add the validation back before processing the field.
Comment on lines
+74
to
+94
| throw new Exception('SQL statement preparation failed: ' . $connNow->error); | ||
| } | ||
|
|
||
| $stmt->bind_param($types, ...$vals); | ||
| if(!$stmt->execute()) throw new Exception('Execute hatası: '.$stmt->error); | ||
| $stmt->bind_param($bindTypes, ...$bindValues); | ||
|
|
||
| if (!$stmt->execute()) { | ||
| throw new Exception('SQL statement execution failed: ' . $stmt->error); | ||
| } | ||
|
|
||
| $resp['success']=true; | ||
| $resp['message']='Araç güncellendi'; | ||
| $resp['affected']=$stmt->affected_rows; | ||
| $response['success'] = true; | ||
| $response['message'] = 'Car info updated successfully.'; | ||
| $response['data'] = ['affected_rows' => $stmt->affected_rows]; | ||
|
|
||
| $stmt->close(); | ||
| } catch(Exception $e){ | ||
| $resp['success']=false; | ||
| $resp['message']=$e->getMessage(); | ||
|
|
||
| } catch (Exception $e) { | ||
| if (http_response_code() === 200) { // If no HTTP code was set by us, it's an internal error | ||
| http_response_code(500); | ||
| } | ||
| $response['success'] = false; | ||
| $response['message'] = $e->getMessage(); |
There was a problem hiding this comment.
This code concatenates $connNow->error and $stmt->error into exception messages that are then returned directly to clients via $response['message'], which can leak internal database details and query structure. An attacker can deliberately trigger SQL errors (for example by sending malformed or out-of-range input) to enumerate schema information and error states useful for further attacks. Instead, log detailed database errors only on the server and return a generic error message to the client while preserving appropriate HTTP status codes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Align the CarInfo edit API response and identifier handling with the expected contract while introducing basic project governance and build updates.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation: