Skip to content

fix: double free WebDAVHandler#1093

Merged
ngxson merged 1 commit intocrosspoint-reader:masterfrom
ngxson:xsn/webdav_double_free
Feb 22, 2026
Merged

fix: double free WebDAVHandler#1093
ngxson merged 1 commit intocrosspoint-reader:masterfrom
ngxson:xsn/webdav_double_free

Conversation

@ngxson
Copy link
Contributor

@ngxson ngxson commented Feb 22, 2026 •
edited
Loading

Summary

Ref: #1047 (comment)

To reproduce:

  1. Open file transfer
  2. Join a network
  3. Once it's connected, press (hold) back

AI Usage

While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it
helps set the right context for reviewers.

Did you use AI tools to help write this code? NO

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 22, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4ccafe5 and d1568bf.

📒 Files selected for processing (2)
  • src/network/CrossPointWebServer.cpp
  • src/network/CrossPointWebServer.h
💤 Files with no reviewable changes (1)
  • src/network/CrossPointWebServer.h
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build
  • GitHub Check: cppcheck
🔇 Additional comments (2)
src/network/CrossPointWebServer.cpp (2)

15-15: LGTM: WebDAVHandler header added.
No concerns with this include change.

163-167: WebDAV handler ownership is properly managed by the WebServer library—no leak exists.

The ESP32 WebServer library automatically takes ownership of handlers registered via addHandler() and deletes them in its destructor. Since CrossPointWebServer::stop() calls server.reset(), which destroys the underlying WebServer instance, the WebDAVHandler is automatically deleted via the WebServer destructor. The code is safe across multiple begin/stop cycles.

The comment in the code ("WebDAVHandler will be deleted by WebServer when server is stopped") accurately reflects the library's behavior.

The optional suggestion to use std::nothrow for allocation is reasonable defensive programming on embedded systems without exceptions, but allocation failure here is unlikely and non-critical for this feature.

📝 Walkthrough

Walkthrough

WebDAV handler lifecycle management is refactored in CrossPointWebServer. The davHandler member variable is removed from the class, and handler registration shifts from using a stack-allocated member to instantiating a new WebDAVHandler object dynamically during server initialization.

Changes

Cohort / File(s) Summary
Header Class Composition
src/network/CrossPointWebServer.h		 Removed WebDAVHandler.h include and private davHandler member variable.
Source Handler Registration
src/network/CrossPointWebServer.cpp		 Added WebDAVHandler.h include and updated handler registration to instantiate WebDAVHandler dynamically via new, shifting ownership to WebServer; added corresponding log message.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main fix: addressing a double free issue with WebDAVHandler by changing from a stack-allocated member to dynamic allocation.
Description check ✅ Passed The description provides context for the fix by referencing a discussion thread and including reproduction steps, directly related to the WebDAVHandler double free bug being addressed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Comment @coderabbitai help to get the list of available commands and usage tips.

osteotek
osteotek approved these changes Feb 22, 2026
View reviewed changes
Copy link
Member

@osteotek osteotek left a comment •
edited
Loading

Choose a reason for hiding this comment

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

fixed the crash for me

@osteotek osteotek mentioned this pull request Feb 22, 2026
Closed
@ngxson ngxson merged commit 75ff7b2 into crosspoint-reader:master Feb 22, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@znelson znelson znelson approved these changes

@osteotek osteotek osteotek approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ngxson @znelson @osteotek