Copilot commented on code in PR #13384: URL: https://github.com/apache/trafficserver/pull/13384#discussion_r3587249755
########## tests/tools/plugins/packet_mark_common.cc: ########## @@ -0,0 +1,186 @@ +/** @file + + Shared helpers for the packet-mark test plugins. + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include "packet_mark_common.h" + +extern "C" { +#include <sys/socket.h> +} + +#include <cerrno> +#include <cstdint> +#include <cstdio> +#include <cstdlib> +#include <optional> +#include <string> + +namespace packet_mark +{ +namespace +{ + // The tsapi setter and getters are bound as non-type template parameters below, + // which pins the correct client/server trio at compile time. These must be raw + // function-pointer types, not std::function: a non-type template parameter has + // to be a structural type, and std::function is a runtime type-erasure wrapper, + // so template <std::function<...> Setter> is ill-formed. + using MarkSetter = TSReturnCode (*)(TSHttpTxn, int); + using FdGetter = TSReturnCode (*)(TSHttpTxn, int *); + using RespGetter = TSReturnCode (*)(TSHttpTxn, TSMBuffer *, TSMLoc *); + + std::optional<uint32_t> + get_uint_header(TSMBuffer bufp, TSMLoc hdr_loc, std::string_view header) + { + // Values are parsed with strtoul (base 0), so "0x0000000A" and "10" are both + // accepted. Returns std::nullopt if the header is absent, empty, or not a valid + // number -- a malformed value is a test-harness error, not a silent 0. */ + TSMLoc field_loc{TSMimeHdrFieldFind(bufp, hdr_loc, header.data(), static_cast<int>(header.length()))}; Review Comment: There is a stray `*/` at the end of this `//` comment, which will cause a compilation error. ########## tests/tools/plugins/packet_mark_common.cc: ########## @@ -0,0 +1,186 @@ +/** @file + + Shared helpers for the packet-mark test plugins. + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include "packet_mark_common.h" + +extern "C" { +#include <sys/socket.h> +} + +#include <cerrno> +#include <cstdint> +#include <cstdio> +#include <cstdlib> +#include <optional> +#include <string> + +namespace packet_mark +{ +namespace +{ + // The tsapi setter and getters are bound as non-type template parameters below, + // which pins the correct client/server trio at compile time. These must be raw + // function-pointer types, not std::function: a non-type template parameter has + // to be a structural type, and std::function is a runtime type-erasure wrapper, + // so template <std::function<...> Setter> is ill-formed. + using MarkSetter = TSReturnCode (*)(TSHttpTxn, int); + using FdGetter = TSReturnCode (*)(TSHttpTxn, int *); + using RespGetter = TSReturnCode (*)(TSHttpTxn, TSMBuffer *, TSMLoc *); + + std::optional<uint32_t> + get_uint_header(TSMBuffer bufp, TSMLoc hdr_loc, std::string_view header) + { + // Values are parsed with strtoul (base 0), so "0x0000000A" and "10" are both + // accepted. Returns std::nullopt if the header is absent, empty, or not a valid + // number -- a malformed value is a test-harness error, not a silent 0. */ + TSMLoc field_loc{TSMimeHdrFieldFind(bufp, hdr_loc, header.data(), static_cast<int>(header.length()))}; + if (field_loc == TS_NULL_MLOC) { + return std::nullopt; + } + + int value_len{0}; + char const *value{TSMimeHdrFieldValueStringGet(bufp, hdr_loc, field_loc, -1, &value_len)}; + + std::optional<uint32_t> result{std::nullopt}; + if (value != nullptr && value_len > 0) { + char *end{nullptr}; + errno = 0; + unsigned long const parsed{std::strtoul(value, &end, 0)}; + // Reject empty, partially-numeric, or out-of-range values: a malformed + // header is a test-harness error, not a silent 0. + if (errno == 0 && end == value + value_len && parsed <= UINT32_MAX) { + result = static_cast<uint32_t>(parsed); + } + } Review Comment: `TSMimeHdrFieldValueStringGet` returns a pointer plus an explicit length; the returned buffer is not guaranteed to be NUL-terminated. Passing it directly to `std::strtoul` can read past the end of the header value (undefined behavior). Copy into a NUL-terminated buffer (or use a length-bounded parse) before calling `strtoul`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
