Skip to content
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

Hardening related to resolving Forms files #436

Merged
merged 7 commits into from
Jan 1, 2024
Merged

Conversation

martinhpedersen
Copy link
Member

Improved error handling, more efficient search and make sure we don't escape the Forms root directory. Also adds debug logging and fixes some reply templates we missed before.

@martinhpedersen martinhpedersen self-assigned this Dec 23, 2023
Improved error handling, more efficient search and make sure we don't
escape the Forms root directory. Also adds debug logging and fixes some
reply templates we missed before.
Copy link
Contributor

@xylo04 xylo04 left a comment

Choose a reason for hiding this comment

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

Nice improvements!

Makes it possible to load templates outside the Standard Forms folder.
Mostly applicable for `composeform --template [path]`.
No need to traverse the whole file system when the exact location to the
.dat file is already known.
@martinhpedersen
Copy link
Member Author

Thanks for the review Chris 😄

I did some more testing over the holiday and spotted some issues related to reading template files outside the Standard Forms directory (i.e. with pat composeform --template ~/foo/bar/.txt). I also identified a couple more weak points and did some more refactoring.

I'll commit further changes to the PR over the next days. Will let you know when I believe it's ready for a new review 🙂

This was making the code more complex, as it made an unnecessary
fragmentation between the cli composer and the web gui. Many of the
fields exposed via the web api was in fact unused.
Fixes ignored MsgTo, MsgBcc and other common fields.
@martinhpedersen martinhpedersen marked this pull request as ready for review December 29, 2023 10:17
@martinhpedersen martinhpedersen changed the base branch from master to develop December 29, 2023 10:17
@martinhpedersen
Copy link
Member Author

Ready for a new review if you don't mind 🙂

I ended up doing a lot more refactoring, and at the same time squashed a couple of bugs. There is still plenty of work to be done, especially regarding support for txt-only templates. However, I think think it's best to do this in smaller increments som it's easier to do reviews and keep track of the changes.

PS: I starting working on this because NRRL (Norwegian Radio Relay League) had a x-mas exercise using templates this week, and I got a lot of questions from Norwegian hams struggling to get things working properly. I realize I need to invest more time in the Forms feature to get it up to the current "standards".

@martinhpedersen martinhpedersen merged commit a040c76 into develop Jan 1, 2024
4 checks passed
@martinhpedersen martinhpedersen deleted the forms-hardening branch January 1, 2024 12:48
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.

2 participants