Skip to content

Conversation

@mikeller
Copy link
Member

@mikeller mikeller commented Oct 31, 2025

Add support for building and serving the German versions of the manual
and mobile manual.

Signed-off-by: Michael Keller [email protected]

@mikeller mikeller requested a review from dirkhh October 31, 2025 07:36
@mikeller mikeller force-pushed the add_german_manuals branch 2 times, most recently from 4b7a37c to e882fba Compare October 31, 2025 09:46
@mikeller mikeller requested a review from Copilot November 1, 2025 00:32
Copy link
Contributor

Copilot AI left a comment

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 adds German language support for user manuals by implementing localized documentation serving based on URL language parameters.

  • Extends manual generation to include German translations (user-manual_de.html and mobile-manual_de.html)
  • Implements language-aware routing in server endpoints to serve localized manuals
  • Updates .gitignore to exclude generated German manual files

Reviewed Changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 5 comments.

File Description
src/web/subsurfacesync.py Extended build and copy operations to include German manual files
src/web/server.py Added language detection logic to serve localized manuals based on lang query parameter
.gitignore Added German manual files to ignore list

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

language = request.args.get("lang")

if language:
file_name = "user-manual_" + language[:2] + ".html"
Copy link

Copilot AI Nov 1, 2025

Choose a reason for hiding this comment

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

The language[:2] slicing without sanitization could allow path traversal attacks. An attacker could pass special characters or path sequences in the lang parameter (e.g., ?lang=../malicious). While os.path.exists() provides some protection, you should validate that language contains only alphanumeric characters before constructing the file name. Consider using language[:2].isalpha() or a regex pattern to validate the input.

Copilot uses AI. Check for mistakes.

if language:
file_name = "user-manual_" + language[:2] + ".html"
file_path = os.path.join(os.path.join(app.root_path, "static"), file_name)
Copy link

Copilot AI Nov 1, 2025

Choose a reason for hiding this comment

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

The nested os.path.join() call is redundant. You can simplify this to os.path.join(app.root_path, 'static', file_name) which is more readable and achieves the same result.

Copilot uses AI. Check for mistakes.
language = request.args.get("lang")

if language:
file_name = "mobile-user-manual_" + language[:2] + ".html"
Copy link

Copilot AI Nov 1, 2025

Choose a reason for hiding this comment

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

The language[:2] slicing without sanitization could allow path traversal attacks. An attacker could pass special characters or path sequences in the lang parameter (e.g., ?lang=../malicious). While os.path.exists() provides some protection, you should validate that language contains only alphanumeric characters before constructing the file name. Consider using language[:2].isalpha() or a regex pattern to validate the input.

Copilot uses AI. Check for mistakes.

if language:
file_name = "mobile-user-manual_" + language[:2] + ".html"
file_path = os.path.join(os.path.join(app.root_path, "static"), file_name)
Copy link

Copilot AI Nov 1, 2025

Choose a reason for hiding this comment

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

The nested os.path.join() call is redundant. You can simplify this to os.path.join(app.root_path, 'static', file_name) which is more readable and achieves the same result.

Copilot uses AI. Check for mistakes.
Comment on lines 182 to 208
@app.route("/subsurface-user-manual/")
def static_user_manual():
language = request.args.get("lang")

if language:
file_name = "user-manual_" + language[:2] + ".html"
file_path = os.path.join(os.path.join(app.root_path, "static"), file_name)
if os.path.exists(file_path):
return send_from_directory(os.path.join(app.root_path, "static"), file_name)

return send_from_directory(os.path.join(app.root_path, "static"), "user-manual.html")


@app.route("/subsurface-mobile-v3-user-manual/")
@app.route("/subsurface-mobile-user-manual/")
def static_mobile_user_manual():
language = request.args.get("lang")

if language:
file_name = "mobile-user-manual_" + language[:2] + ".html"
file_path = os.path.join(os.path.join(app.root_path, "static"), file_name)
if os.path.exists(file_path):
return send_from_directory(os.path.join(app.root_path, "static"), file_name)

return send_from_directory(os.path.join(app.root_path, "static"), "mobile-user-manual.html")


Copy link

Copilot AI Nov 1, 2025

Choose a reason for hiding this comment

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

The logic for language-based file serving in static_user_manual() and static_mobile_user_manual() is duplicated. Consider extracting this into a helper function that takes the base filename as a parameter to reduce code duplication and improve maintainability.

Suggested change
@app.route("/subsurface-user-manual/")
def static_user_manual():
language = request.args.get("lang")
if language:
file_name = "user-manual_" + language[:2] + ".html"
file_path = os.path.join(os.path.join(app.root_path, "static"), file_name)
if os.path.exists(file_path):
return send_from_directory(os.path.join(app.root_path, "static"), file_name)
return send_from_directory(os.path.join(app.root_path, "static"), "user-manual.html")
@app.route("/subsurface-mobile-v3-user-manual/")
@app.route("/subsurface-mobile-user-manual/")
def static_mobile_user_manual():
language = request.args.get("lang")
if language:
file_name = "mobile-user-manual_" + language[:2] + ".html"
file_path = os.path.join(os.path.join(app.root_path, "static"), file_name)
if os.path.exists(file_path):
return send_from_directory(os.path.join(app.root_path, "static"), file_name)
return send_from_directory(os.path.join(app.root_path, "static"), "mobile-user-manual.html")
def _serve_user_manual(base_filename):
language = request.args.get("lang")
static_dir = os.path.join(app.root_path, "static")
if language:
file_name = f"{base_filename}_{language[:2]}.html"
file_path = os.path.join(static_dir, file_name)
if os.path.exists(file_path):
return send_from_directory(static_dir, file_name)
return send_from_directory(static_dir, f"{base_filename}.html")
@app.route("/subsurface-user-manual/")
def static_user_manual():
return _serve_user_manual("user-manual")
@app.route("/subsurface-mobile-v3-user-manual/")
@app.route("/subsurface-mobile-user-manual/")
def static_mobile_user_manual():
return _serve_user_manual("mobile-user-manual")

Copilot uses AI. Check for mistakes.
Add support for building and serving the German versions of the manual
and mobile manual.

Signed-off-by: Michael Keller <[email protected]>
@mikeller mikeller requested a review from Copilot November 1, 2025 01:44
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +186 to +190
if language and language[:2].isalpha():
file_name = f"{base_filename}_{language[:2]}.html"
file_path = os.path.join(static_dir, file_name)
if os.path.exists(file_path):
return send_from_directory(static_dir, file_name)
Copy link

Copilot AI Nov 1, 2025

Choose a reason for hiding this comment

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

The language parameter validation is insufficient and may allow path traversal attacks. While isalpha() checks if characters are alphabetic, it doesn't prevent sequences like ../. Use a whitelist approach by checking against the languages list defined in the module (lines 39-67), or at minimum use language[:2].isalpha() and language[:2].islower() and verify there are no special characters in the full language string before using it in file operations.

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant