Skip to content

Commit ac4a744

Browse files
yfeldblummeta-codesync[bot]
authored andcommitted
use explicit types in Cursor::write in proxygen/
Summary: Addresses problems like this: ``` void* buf = /*...*/; uint16_t word = /*...*/; cursor.write(word & 0xffff); // oops, 32-bit store due to implicit integer promotion ``` Differential Revision: D94252977 fbshipit-source-id: 3d3645d298662bf7a8e79727b3b84cb35b51ec69
1 parent 2445098 commit ac4a744

14 files changed

Lines changed: 56 additions & 43 deletions

File tree

third-party/proxygen/src/proxygen/httpserver/samples/hq/devious/DeviousBaton.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ namespace {
1717
std::unique_ptr<folly::IOBuf> makeBatonMessage(uint64_t padLen, uint8_t baton) {
1818
auto buf = folly::IOBuf::create(padLen + 9);
1919
folly::io::Appender cursor(buf.get(), 1);
20-
(void)quic::encodeQuicInteger(padLen, [&](auto val) { cursor.writeBE(val); });
20+
(void)quic::encodeQuicInteger(padLen, [&](auto val) {
21+
cursor.writeBE(folly::tag<decltype(val)>, val);
22+
});
2123
memset(buf->writableTail(), 'a', padLen);
2224
buf->append(padLen);
2325
buf->writableTail()[0] = baton;

third-party/proxygen/src/proxygen/lib/http/codec/HQFramer.cpp

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ WriteResult writeFrameHeader(IOBufQueue& queue,
199199
uint64_t length) noexcept {
200200
QueueAppender appender(&queue, kMaxFrameHeaderSize);
201201
auto appenderOp = [appender = std::move(appender)](auto val) mutable {
202-
appender.writeBE(val);
202+
appender.writeBE(folly::tag<decltype(val)>, val);
203203
};
204204
auto typeRes =
205205
quic::encodeQuicInteger(static_cast<uint64_t>(type), appenderOp);
@@ -251,7 +251,7 @@ WriteResult writeCancelPush(folly::IOBufQueue& writeBuf,
251251
QueueAppender appender(&queue, *pushIdSize);
252252
auto encodeResult = quic::encodeQuicInteger(
253253
pushId, [appender = std::move(appender)](auto val) mutable {
254-
appender.writeBE(val);
254+
appender.writeBE(folly::tag<decltype(val)>, val);
255255
});
256256
if (encodeResult.hasError()) {
257257
return folly::makeUnexpected(quic::QuicError(encodeResult.error()));
@@ -283,7 +283,7 @@ WriteResult writeSettings(IOBufQueue& queue,
283283
// write the frame payload
284284
QueueAppender appender(&queue, settingsSize);
285285
auto appenderOp = [appender = std::move(appender)](auto val) mutable {
286-
appender.writeBE(val);
286+
appender.writeBE(folly::tag<decltype(val)>, val);
287287
};
288288
for (const auto& setting : settings) {
289289
auto idResult = quic::encodeQuicInteger(
@@ -314,8 +314,9 @@ WriteResult writePushPromise(IOBufQueue& queue,
314314
return headerSize;
315315
}
316316
QueueAppender appender(&queue, payloadSize);
317-
auto encodeResult =
318-
quic::encodeQuicInteger(pushId, [&](auto val) { appender.writeBE(val); });
317+
auto encodeResult = quic::encodeQuicInteger(pushId, [&](auto val) {
318+
appender.writeBE(folly::tag<decltype(val)>, val);
319+
});
319320
if (encodeResult.hasError()) {
320321
return folly::makeUnexpected(quic::QuicError(encodeResult.error()));
321322
}
@@ -333,7 +334,7 @@ WriteResult writeGoaway(folly::IOBufQueue& writeBuf,
333334
QueueAppender appender(&queue, *lastStreamIdSize);
334335
auto encodeResult = quic::encodeQuicInteger(
335336
lastStreamId, [appender = std::move(appender)](auto val) mutable {
336-
appender.writeBE(val);
337+
appender.writeBE(folly::tag<decltype(val)>, val);
337338
});
338339
if (encodeResult.hasError()) {
339340
return folly::makeUnexpected(quic::QuicError(encodeResult.error()));
@@ -351,7 +352,7 @@ WriteResult writeMaxPushId(folly::IOBufQueue& writeBuf,
351352
QueueAppender appender(&queue, *maxPushIdSize);
352353
auto encodeResult = quic::encodeQuicInteger(
353354
maxPushId, [appender = std::move(appender)](auto val) mutable {
354-
appender.writeBE(val);
355+
appender.writeBE(folly::tag<decltype(val)>, val);
355356
});
356357
if (encodeResult.hasError()) {
357358
return folly::makeUnexpected(quic::QuicError(encodeResult.error()));
@@ -369,8 +370,9 @@ WriteResult writePriorityUpdate(folly::IOBufQueue& writeBuf,
369370
}
370371
IOBufQueue queue(IOBufQueue::cacheChainLength());
371372
QueueAppender appender(&queue, *streamIdSize);
372-
auto encodeResult = quic::encodeQuicInteger(
373-
streamId, [&appender](auto val) { appender.writeBE(val); });
373+
auto encodeResult = quic::encodeQuicInteger(streamId, [&appender](auto val) {
374+
appender.writeBE(folly::tag<decltype(val)>, val);
375+
});
374376
if (encodeResult.hasError()) {
375377
return folly::makeUnexpected(quic::QuicError(encodeResult.error()));
376378
}
@@ -390,8 +392,9 @@ WriteResult writePushPriorityUpdate(
390392
}
391393
IOBufQueue queue(IOBufQueue::cacheChainLength());
392394
QueueAppender appender(&queue, *streamIdSize);
393-
auto encodeResult = quic::encodeQuicInteger(
394-
pushId, [&appender](auto val) { appender.writeBE(val); });
395+
auto encodeResult = quic::encodeQuicInteger(pushId, [&appender](auto val) {
396+
appender.writeBE(folly::tag<decltype(val)>, val);
397+
});
395398
if (encodeResult.hasError()) {
396399
return folly::makeUnexpected(quic::QuicError(encodeResult.error()));
397400
}
@@ -407,8 +410,10 @@ WriteResult writeStreamPreface(folly::IOBufQueue& writeBuf,
407410
return folly::makeUnexpected(streamPrefaceSize.error());
408411
}
409412
QueueAppender appender(&writeBuf, *streamPrefaceSize);
410-
auto encodeResult = quic::encodeQuicInteger(
411-
streamPreface, [&appender](auto val) { appender.writeBE(val); });
413+
auto encodeResult =
414+
quic::encodeQuicInteger(streamPreface, [&appender](auto val) {
415+
appender.writeBE(folly::tag<decltype(val)>, val);
416+
});
412417
if (encodeResult.hasError()) {
413418
return folly::makeUnexpected(quic::QuicError(encodeResult.error()));
414419
}
@@ -477,14 +482,16 @@ WriteResult writeWTStreamPreface(folly::IOBufQueue& writeBuf,
477482
CHECK_LT(idx, streamTypes.size());
478483
QueueAppender appender(&writeBuf, 64);
479484
size_t prefaceSize = 0;
480-
auto res = quic::encodeQuicInteger(
481-
streamTypes[idx], [&appender](auto val) { appender.writeBE(val); });
485+
auto res = quic::encodeQuicInteger(streamTypes[idx], [&appender](auto val) {
486+
appender.writeBE(folly::tag<decltype(val)>, val);
487+
});
482488
if (!res) {
483489
return folly::makeUnexpected(quic::QuicError(res.error()));
484490
}
485491
prefaceSize += res.value();
486-
res = quic::encodeQuicInteger(
487-
wtSessionId, [&appender](auto val) { appender.writeBE(val); });
492+
res = quic::encodeQuicInteger(wtSessionId, [&appender](auto val) {
493+
appender.writeBE(folly::tag<decltype(val)>, val);
494+
});
488495
if (!res) {
489496
return folly::makeUnexpected(quic::QuicError(res.error()));
490497
}

third-party/proxygen/src/proxygen/lib/http/codec/HTTP2Framer.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -822,9 +822,9 @@ size_t writeAltSvc(IOBufQueue& queue,
822822
QueueAppender appender(&queue, frameLen);
823823
appender.writeBE<uint32_t>(maxAge);
824824
appender.writeBE<uint16_t>(port);
825-
appender.writeBE<uint8_t>(protoLen);
825+
appender.writeBE<uint8_t>(static_cast<uint8_t>(protoLen));
826826
appender.push(reinterpret_cast<const uint8_t*>(protocol.data()), protoLen);
827-
appender.writeBE<uint8_t>(hostLen);
827+
appender.writeBE<uint8_t>(static_cast<uint8_t>(hostLen));
828828
appender.push(reinterpret_cast<const uint8_t*>(host.data()), hostLen);
829829
appender.push(reinterpret_cast<const uint8_t*>(origin.data()), originLen);
830830
return kFrameHeaderSize + frameLen;

third-party/proxygen/src/proxygen/lib/http/codec/HTTPBinaryCodec.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ namespace proxygen {
2727
namespace {
2828
folly::Expected<size_t, quic::TransportErrorCode> encodeInteger(
2929
uint64_t i, folly::io::QueueAppender& appender) {
30-
auto result =
31-
quic::encodeQuicInteger(i, [&](auto val) { appender.writeBE(val); });
30+
auto result = quic::encodeQuicInteger(
31+
i, [&](auto val) { appender.writeBE(folly::tag<decltype(val)>, val); });
3232
if (result.has_value()) {
3333
return result.value();
3434
} else {

third-party/proxygen/src/proxygen/lib/http/codec/compress/experimental/interop/QPACKInterop.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ void writeFrame(folly::io::QueueAppender& appender,
3737
uint64_t streamId,
3838
std::unique_ptr<folly::IOBuf> buf) {
3939
appender.writeBE<uint64_t>(streamId);
40-
appender.writeBE<uint32_t>(buf->computeChainDataLength());
40+
appender.writeBE<uint32_t>(
41+
static_cast<uint32_t>(buf->computeChainDataLength()));
4142
appender.insert(std::move(buf));
4243
}
4344

third-party/proxygen/src/proxygen/lib/http/codec/compress/experimental/simulator/HPACKScheme.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class HPACKScheme : public CompressionScheme {
4848
auto block = client_.encode(allHeaders);
4949
block->prepend(sizeof(uint16_t));
5050
folly::io::RWPrivateCursor c(block.get());
51-
c.writeBE<uint16_t>(index++);
51+
c.writeBE<uint16_t>(static_cast<uint16_t>(index++));
5252
stats.uncompressed += client_.getEncodedSize().uncompressed;
5353
stats.compressed += client_.getEncodedSize().compressed;
5454
// OOO is allowed with 0 table size

third-party/proxygen/src/proxygen/lib/http/codec/compress/experimental/simulator/QPACKScheme.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,12 @@ class QPACKScheme : public CompressionScheme {
9898
// Don't count the framing against the compression ratio, for now
9999
// stats.compressed += 3 * sizeof(uint16_t);
100100
} else {
101-
cursor.writeBE<uint16_t>(0);
101+
cursor.writeBE<uint16_t>(static_cast<uint16_t>(0));
102102
}
103103
if (result.stream) {
104104
len = result.stream->computeChainDataLength();
105105
}
106-
cursor.writeBE<uint16_t>(index);
106+
cursor.writeBE<uint16_t>(static_cast<uint16_t>(index));
107107
cursor.writeBE<uint16_t>(len);
108108
cursor.insert(std::move(result.stream));
109109
stats.uncompressed += client_.getEncodedSize().uncompressed;

third-party/proxygen/src/proxygen/lib/http/codec/test/HQFramerTest.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,8 @@ TEST_P(HQFramerTestIdOnlyFrames, TestIdOnlyFrame) {
262262
RWPrivateCursor wcursor(buf.get());
263263
// 2 bytes frame header (payload length is just 1)
264264
wcursor.skip(2);
265-
wcursor.writeBE<uint8_t>(0x42); // this varint requires two bytes
265+
wcursor.writeBE<uint8_t>( // this varint requires two bytes
266+
static_cast<uint8_t>(0x42));
266267
queue_.append(std::move(buf));
267268

268269
FrameHeader header;

third-party/proxygen/src/proxygen/lib/http/codec/test/HTTP2CodecTest.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1267,7 +1267,7 @@ TEST_F(HTTP2CodecTest, ZeroWindow) {
12671267
upstreamCodec_.generateWindowUpdate(output_, streamID, 1);
12681268
output_.trimEnd(http2::kFrameWindowUpdateSize);
12691269
QueueAppender appender(&output_, http2::kFrameWindowUpdateSize);
1270-
appender.writeBE<uint32_t>(0);
1270+
appender.writeBE<uint32_t>(static_cast<uint32_t>(0));
12711271

12721272
parse();
12731273
// This test doesn't ensure that RST_STREAM is generated
@@ -1677,7 +1677,7 @@ TEST_F(HTTP2CodecTest, BadRFC9218Priority) {
16771677
EXPECT_TRUE(parse([&](IOBuf* ingress) {
16781678
folly::io::RWPrivateCursor c(ingress);
16791679
c.skip(http2::kFrameHeaderSize + sizeof(uint32_t));
1680-
c.writeBE<uint32_t>(1);
1680+
c.writeBE<uint32_t>(static_cast<uint32_t>(1));
16811681
}));
16821682

16831683
// ill-formatted priority, so returns default priority

third-party/proxygen/src/proxygen/lib/http/codec/test/HTTP2FramerTest.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ TEST_F(HTTP2FramerTest, GoawayDoubleRead) {
260260
queue_, kFrameGoawaySize, static_cast<uint8_t>(FrameType::GOAWAY), 0, 0);
261261

262262
QueueAppender appender(&queue_, kFrameGoawaySize);
263-
appender.writeBE<uint32_t>(0);
263+
appender.writeBE<uint32_t>(static_cast<uint32_t>(0));
264264
// Here's the invalid value:
265265
appender.writeBE<uint32_t>(static_cast<uint32_t>(0xffffffff));
266266

@@ -413,7 +413,7 @@ TEST_F(HTTP2FramerTest, ShortCertificateRequest) {
413413
writeFrameHeaderManual(
414414
queue_, 1, static_cast<uint8_t>(FrameType::CERTIFICATE_REQUEST), 0, 0);
415415
QueueAppender appender(&queue_, 1);
416-
appender.writeBE<uint8_t>(1);
416+
appender.writeBE<uint8_t>(static_cast<uint8_t>(1));
417417

418418
Cursor cursor(queue_.front());
419419
FrameHeader header;
@@ -430,7 +430,7 @@ TEST_F(HTTP2FramerTest, CertificateRequestOnNonzeroStream) {
430430
writeFrameHeaderManual(
431431
queue_, 2, static_cast<uint8_t>(FrameType::CERTIFICATE_REQUEST), 0, 1);
432432
QueueAppender appender(&queue_, 1);
433-
appender.writeBE<uint16_t>(1);
433+
appender.writeBE<uint16_t>(static_cast<uint16_t>(1));
434434

435435
Cursor cursor(queue_.front());
436436
FrameHeader header;
@@ -509,7 +509,7 @@ TEST_F(HTTP2FramerTest, ShortCertificate) {
509509
writeFrameHeaderManual(
510510
queue_, 1, static_cast<uint8_t>(FrameType::CERTIFICATE), 0, 0);
511511
QueueAppender appender(&queue_, 1);
512-
appender.writeBE<uint8_t>(1);
512+
appender.writeBE<uint8_t>(static_cast<uint8_t>(1));
513513

514514
Cursor cursor(queue_.front());
515515
FrameHeader header;
@@ -525,7 +525,7 @@ TEST_F(HTTP2FramerTest, CertificateOnNonzeroStream) {
525525
writeFrameHeaderManual(
526526
queue_, 2, static_cast<uint8_t>(FrameType::CERTIFICATE), 0, 1);
527527
QueueAppender appender(&queue_, 1);
528-
appender.writeBE<uint16_t>(1);
528+
appender.writeBE<uint16_t>(static_cast<uint16_t>(1));
529529

530530
Cursor cursor(queue_.front());
531531
FrameHeader header;

0 commit comments

Comments
 (0)