Skip to content

Conversation

@speeddragon
Copy link

When a URL's size matches the 32-byte binary hash, we don't want it converted to an ID and shortened.

@speeddragon speeddragon changed the title impr: Do not short URL binaries impr: Avoid confusion of 32 bytes URL with 32 bytes hash Nov 12, 2025
@speeddragon
Copy link
Author

Tests pass, but there are some failed/flaky test that exists in edge:

FAIL

  • hb_http:ans104_wasm_test
  • dev_codec_ans104:unsigned_mixedcase_bundle_map_tags_test
  • dev_codec_ans104:unsigned_mixedcase_bundle_list_tags_2_test
  • dev_coded_ans104:unsigned_mixedcase_bundle_list_tags_1_test

FLAKY

  • hb_store_gateway: remote_hyperbeam_node_ans104_test
  • dev_router:strategy_suite_test_/0-fun-1- (Nearest: choose 1)
  • dev_router:dynamic_routing_by_performance
  • dev_meta:config_test

@samcamwilliams
Copy link
Collaborator

I am assuming this is to make the presentation layer of the pretty-printer nicer? If so, maybe we put the cases there instead in something like:

maybe_shorten(URL = <<"http:", _>>) -> URL;
maybe_shorten(URL = <<"https:", _>>) -> URL;
maybe_shorten(Bin) -> hb_util:short_id(Bin).

short_id is actually used in the functional path of a few devices IIRC (codecs, JSON-iface, etc). We should avoid that if you have a target: http://... that happens to be 43 chars, it gets rekt. Only a vague memory, but I think I remember there being a target key with a URL somewhere in the system.

@speeddragon
Copy link
Author

I am assuming this is to make the presentation layer of the pretty-printer nicer? If so, maybe we put the cases there instead in something like:

maybe_shorten(URL = <<"http:", _>>) -> URL;
maybe_shorten(URL = <<"https:", _>>) -> URL;
maybe_shorten(Bin) -> hb_util:short_id(Bin).

short_id is actually used in the functional path of a few devices IIRC (codecs, JSON-iface, etc). We should avoid that if you have a target: http://... that happens to be 43 chars, it gets rekt. Only a vague memory, but I think I remember there being a target key with a URL somewhere in the system.

I've checked the use cases for short_id, and I don't think it is used outside of logging and display message functionality. Inside short_id, hb_util:human_id is called, so this looks already like a similar abstraction as the one you propose.

@samcamwilliams samcamwilliams merged commit 825b35e into edge Nov 19, 2025
@samcamwilliams samcamwilliams deleted the do_not_short_url branch November 19, 2025 00:23
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.

3 participants