-
Notifications
You must be signed in to change notification settings - Fork 0
Linters 2 #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Linters 2 #12
Changes from all commits
639eafb
791cad9
29217a7
746a5a1
9db563f
b2c002c
fd59353
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -90,4 +90,57 @@ def wow_outofstock_api(): | |||||||||||
fieldnames=fieldnames, | ||||||||||||
len=len, | ||||||||||||
) | ||||||||||||
) | ||||||||||||
|
||||||||||||
@wow_bp.route("/petshoppinglist", methods=["GET", "POST"]) | ||||||||||||
def petshoppinglist(): | ||||||||||||
# return redirect("https://saddlebagexchange.com/wow/shopping-list") | ||||||||||||
|
||||||||||||
# DEPRECIATED | ||||||||||||
if request.method == "GET": | ||||||||||||
return return_safe_html(render_template("petshoppinglist.html")) | ||||||||||||
elif request.method == "POST": | ||||||||||||
json_data = { | ||||||||||||
"region": request.form.get("region"), | ||||||||||||
"itemID": int(request.form.get("petID")), | ||||||||||||
"maxPurchasePrice": int(request.form.get("maxPurchasePrice")), | ||||||||||||
"connectedRealmIDs": {}, | ||||||||||||
} | ||||||||||||
Comment on lines
+103
to
+108
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add input validation for form data The route accepts form data without validation. Consider adding proper validation: + from typing import Optional
+ from dataclasses import dataclass
+ from flask import abort
+ @dataclass
+ class ShoppingListRequest:
+ region: str
+ item_id: int
+ max_purchase_price: int
+
+ @classmethod
+ def from_form(cls, form_data) -> Optional['ShoppingListRequest']:
+ try:
+ return cls(
+ region=form_data.get('region'),
+ item_id=int(form_data.get('petID')),
+ max_purchase_price=int(form_data.get('maxPurchasePrice'))
+ )
+ except (ValueError, TypeError):
+ return None
def petshoppinglist():
if request.method == "POST":
+ data = ShoppingListRequest.from_form(request.form)
+ if not data:
+ abort(400, description="Invalid input data")
json_data = {
- "region": request.form.get("region"),
- "itemID": int(request.form.get("petID")),
- "maxPurchasePrice": int(request.form.get("maxPurchasePrice")),
+ "region": data.region,
+ "itemID": data.item_id,
+ "maxPurchasePrice": data.max_purchase_price,
"connectedRealmIDs": {},
}
|
||||||||||||
|
||||||||||||
print(json_data) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Replace print statements with proper logging Debug print statements should be replaced with proper logging: + import logging
+ logger = logging.getLogger(__name__)
- print(json_data)
+ logger.debug("Processing shopping list request with data: %s", json_data)
- print(response)
+ logger.debug("Received API response: %s", response)
- print(f"Error no matching data with given inputs {json_data} response {response}")
+ logger.error("No matching data found. Inputs: %s, Response: %s", json_data, response) Also applies to: 118-118, 121-123 |
||||||||||||
|
||||||||||||
response = requests.post( | ||||||||||||
f"{api_url}/wow/shoppinglistx", | ||||||||||||
headers={"Accept": "application/json"}, | ||||||||||||
json=json_data, | ||||||||||||
).json() | ||||||||||||
|
||||||||||||
print(response) | ||||||||||||
|
||||||||||||
if "data" not in response: | ||||||||||||
print( | ||||||||||||
f"Error no matching data with given inputs {json_data} response {response}" | ||||||||||||
) | ||||||||||||
if NO_RATE_LIMIT: | ||||||||||||
return f"Error no matching data with given inputs {json_data} response {response}" | ||||||||||||
# send generic error message to remove XSS potential | ||||||||||||
return f"error no matching results found matching search inputs" | ||||||||||||
Comment on lines
+124
to
+127
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Security: Remove detailed error exposure When NO_RATE_LIMIT is True, the error message exposes internal data (json_data and response) which could contain sensitive information. - if NO_RATE_LIMIT:
- return f"Error no matching data with given inputs {json_data} response {response}"
- # send generic error message to remove XSS potential
- return f"error no matching results found matching search inputs"
+ return "No matching results found for the given search criteria" 📝 Committable suggestion
Suggested change
|
||||||||||||
|
||||||||||||
response = response["data"] | ||||||||||||
|
||||||||||||
column_order = [ | ||||||||||||
"realmID", | ||||||||||||
"price", | ||||||||||||
"quantity", | ||||||||||||
"realmName", | ||||||||||||
"realmNames", | ||||||||||||
"link", | ||||||||||||
] | ||||||||||||
response = [{key: item.get(key) for key in column_order} for item in response] | ||||||||||||
fieldnames = list(response[0].keys()) | ||||||||||||
|
||||||||||||
return return_safe_html( | ||||||||||||
render_template( | ||||||||||||
"petshoppinglist.html", results=response, fieldnames=fieldnames, len=len | ||||||||||||
) | ||||||||||||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove or update deprecated route
The route is marked as deprecated but still being added. The commented redirect suggests moving to a new URL.
Either:
📝 Committable suggestion