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

ble/skald: Always enforce URL length check #20577

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

Teufelchen1
Copy link
Contributor

Contribution description

Call for feedback: Should we assume somebody builds an app where the URL parameter is not a compile time constant?
If so, we need to check the length even when assertions are disabled.

@github-actions github-actions bot added Area: network Area: Networking Area: BLE Area: Bluetooth Low Energy support Area: sys Area: System labels Apr 15, 2024
sys/net/ble/skald/skald_eddystone.c Outdated Show resolved Hide resolved
@benpicco benpicco added Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 15, 2024
@miri64
Copy link
Member

miri64 commented Apr 15, 2024

Call for feedback: Should we assume somebody builds an app where the URL parameter is not a compile time constant?

IMHO, since Skald is just fullfils a “push my website's URL to some persons phone” use case, I'd say it is totally fine to assume that the URL parameter is a compile-time constant. You probably want physical access to the device anyways if you need to change that URL.

@riot-ci
Copy link

riot-ci commented Apr 15, 2024

Murdock results

✔️ PASSED

da79068 ble/skald: Always enforce URL length check

Success Failures Total Runtime
10038 0 10042 13m:47s

Artifacts

@benpicco benpicco added this pull request to the merge queue Apr 15, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Apr 16, 2024
@maribu maribu added this pull request to the merge queue Apr 16, 2024
@maribu maribu removed this pull request from the merge queue due to a manual request Apr 16, 2024
@maribu maribu added this pull request to the merge queue Apr 16, 2024
Merged via the queue into RIOT-OS:master with commit d708f8d Apr 16, 2024
27 checks passed
@mguetschow mguetschow added this to the Release 2024.07 milestone Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: BLE Area: Bluetooth Low Energy support Area: network Area: Networking Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants