Merge BoringSSL through 54821d806d574dd8f2869a8c7f5725b65a67af42 #2275
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gen_group wasn't copied over. Change-Id: If5341dce69fe0297b6bd9a5fb7ed34d546201604 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67167 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
Bug: 660, b:323560158 Change-Id: I1154fb848de28fd0417660cce1f99e3d29107840 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66327 Commit-Queue: Bob Beck <[email protected]> Reviewed-by: David Benjamin <[email protected]>
OpenSSH needs this. Features that have been intentionally omitted for
now:
- X9.42-style Diffie-Hellman ("DHX"). We continue not to support this.
Use ECDH or X25519 instead.
- SPKI and PKCS#8 serialization. Use ECDH or X25519 instead. The format
is a bit ill-defined. Moreover, until we solve the serialization
aspects of https://crbug.com/boringssl/497, adding them would put this
legacy algorithm on path for every caller.
- Most of the random options like stapling a KDF, etc. Though I did add
EVP_PKEY_CTX_set_dh_pad because it's the only way to undo OpenSSL's
bug where they chop off leading zeros by default.
- Parameter generation. Diffie-Hellman parameters should not be
generated at runtime.
This means you need to bootstrap with a DH object and then wrap it in an
EVP_PKEY. This matches the limitations of the EVP API in OpenSSL 1.1.x.
Unfortunately the OpenSSL 3.x APIs are unsuitable for many, many
reasons, so I expect when we get further along in
https://crbug.com/boringssl/535, we'll have established some patterns
here that we can apply to EVP_PKEY_DH too.
Change-Id: I34b4e8799afb266ea5602a70115cc2146f19c6a7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67207
Reviewed-by: Theo Buehler <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Since we are saying this will die when standardized, let us ensure users of this code from this location take notice and action before using it. We then selectively allow it in the speed tool and in our tests. If we like this approach, I'll go back and apply it to kyber (which will have some other fallout consequences to fix) but this one should be painless right now. This can also be applied to Dilithium when it comes back. Future experimentals could be added in this manner. Change-Id: Ie3b41cf16278868562ef1c8b28f2caed5e0e2dd1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66887 Commit-Queue: Bob Beck <[email protected]> Reviewed-by: David Benjamin <[email protected]>
This aligns with the generated build, where fips_shared_support.c is part of crypto_sources, not the delocated part. It is conceptually part of bcm, but our generated builds currently only separate on basis of delocated/partial-linked vs. not delocated. Bug: 542 Change-Id: Ib8de3fb0a7778c9000c3b4fca978d43cb9a29d12 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67267 Reviewed-by: Adam Langley <[email protected]> Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
Change-Id: Ie16e9ab0897305089672720efa4530d43074f692 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67387 Auto-Submit: Theo Buehler <[email protected]> Reviewed-by: Bob Beck <[email protected]> Reviewed-by: David Benjamin <[email protected]> Commit-Queue: Bob Beck <[email protected]>
Lots of code relies on this, so we ought to document it. A null STACK_OF(T) is treated as an immutable empty list. Change-Id: I10d0ba8f7b33c7f3febaf92c2cd3da25a0eb0f80 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67407 Reviewed-by: Theo Buehler <[email protected]> Auto-Submit: David Benjamin <[email protected]> Commit-Queue: Bob Beck <[email protected]> Reviewed-by: Bob Beck <[email protected]>
This is one step towards flattening the libcrypto build into the top-level CMakeLists.txt file. (This is to align the CMake build with our other builds, and also make it easier to consume a CRYPTO_SOURCES variable without having to prepend "../" in front of each value.) This also avoids a rather annoying corner of CMake: files generated in a different directory don't work well, which is why we had all this mess with EXTERNAL_OBJECT, GENERATED, and bcm_o_target. Globbing it into one file is a bit unwieldy, but avoids this. (CMake is incredibly bad at custom rules.) Bug: 542 Change-Id: Ia5038511af339a0eae2af56875a42581eb1ed15b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67287 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
This adds a tool for managing pre-generated files, aligning our CMake and non-CMake builds. The plan is roughly: The source of truth for the file lists will (eventually) be build.json. This describes the build in terms of the files that we directly edit. However, we have a two-phase build. First a pregeneration step transforms some of the less convenient inputs into checked in files. Notably perlasm files get expanded. This produces an equivalent JSON structure with fewer inputs. The same tool then outputs that structure into whatever build systems we want. This initial version pre-generates err_data.c and perlasm files. I've not wired up the various build formats, except for CMake (for the CMake build to consume) and JSON (for generate_build_files.py to parse). build.json is also, for now, only a subset of the build. Later changes The upshot of all this is we no longer have a Perl build dependency! Perl is now only needed when working on BoringSSL. It nearly removes the Go one, but Go is still needed to run and (for now) build the tests. To keep the generated files up-to-date, once this lands, I'll update our CI to run `go run ./util/pregenerate -check` which asserts that all generated files are correct. From there we can land the later changes in this patch series that uses this more extensively. My eventual goal is to replace generate_build_files.py altogether and the "master-with-bazel" branch. Instead we'll just have sources.bzl, sources.gni, etc. all checked into the tree directly. And then the normal branch will just have both a CMake and Bazel build in it. Update-Note: generate_build_files.py no longer generates assembly files or err_data.c. Those are now checked into the tree directly. Bug: 542 Change-Id: I71f5ff7417be811f8b7888b345279474e6b38ee9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67288 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
The error list was updated in between when I uploaded and landed the generated version. Bug: 542 Change-Id: I4d0efdca20264fd2a6508dd8ff4065bd903d5a79 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67428 Auto-Submit: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
Bug: 542 Change-Id: Idd5df8cc485deae6a2fde142854e24e7fecd0520 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67289 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
This is primarily in preparation for pki_test's data list. That thing is 80% of source.cmake. glob patterns are normally not great, but since we're checking the result in, that should be fine. Bug: 542 Change-Id: I6ccf69f4a2ce08b153de5eb9dfb2f9b01654e1ce Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67290 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
Since we can now handle glob patterns, make those shorter. Bug: 542 Change-Id: I971d9785bce0db7b2e8c41c8c82468afde64540d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67292 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
We now have (some of) our test sources in an easily parseable form. run_android_tests.go no longer needs to crawl the source tree. Note this required fixing the .gitignore rules. If a .gitignore line doesn't have a slash at the start or middle, it applies to subdirectories as well. This is confusing, so I just stuck a leading slash in front of all of them. Bug: 681 Change-Id: I389c2a0560594fbd23c60b5b614b0ccfedf28926 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67293 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
Bug: 542 Change-Id: I23c3c5c01ae41bd98f605b34e09269a6602a2c49 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67294 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
We originally embedded test data because of deficiencies in Android's build. Android had no way to specify test data with tests. That has since been resolved, and the embedding mechanism has gotten unwieldy. This unifies pki_test and crypto_test's test data story, and does so in a way that all tests can participate in. (We can now use FileTest in decrepit_test.) Update-Note: This will require some tweaks to downstream builds. We no longer emit an (unwieldy) crypto_test_data.cc file. Instead, tests will expect test data be available at the current working directory. This can be overridden with the BORINGSSL_TEST_DATA_ROOT environment variable. Callers with more complex needs can build with BORINGSSL_CUSTOM_GET_TEST_DATA and then link in an alternate implementation of this function. On the off chance some project needs it, I've kept the embed_test_data.go script around for now, but I expect we can delete it in the future. Fixed: 681 Change-Id: If181ce043e1eea3148838f1bb4db9ee4bfda0d08 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67295 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
The CMake build doesn't use it, but this will be needed for the other builds to be driven by util/pregenerate. Bug: 542 Change-Id: If95cbcef1803e30ffc5ab7c9227fdcc6c53adf34 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67296 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
Bug: 542 Change-Id: Ie5590a0a68431903587fa87d70c1acac4161e9b9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67297 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
This avoids needing to rebase the source lists. It also means that libcrypto.a and libssl.a end up directly in the build directory, which makes it a bit easier to pass it to, say, gcc -L when testing things. That file is, alas, getting a bit large. delocate is a pretty large amount of code. I tried to abstract things into functions to toss into a cmake/delocate.cmake, but CMake is really bad at making abstractions. Bug: 542 Change-Id: I084d7a6bdd4c21ac27859b8b0c9d7a84829f2823 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67298 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
CMake doesn't use these, but a lot of our other build systems need it. Bug: 542 Change-Id: I74388751b832921ac121abd3d5755880f352a449 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67299 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
This is sliiiightly weird because we can't quite decide whether these are headers with the wrong extension or standalone C files. But since most of our build does the former, I've done this for now. I expect we'll need to iterate on this one a bit. Bug: 542 Change-Id: Ib50332c0804efb5a1aa37fe445f129156260835a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67300 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
Change-Id: I3663a5efe1f71192e69e3e04821a481043d145bb Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67467 Commit-Queue: Aaron Knobloch <[email protected]> Reviewed-by: David Benjamin <[email protected]> Auto-Submit: Aaron Knobloch <[email protected]> Commit-Queue: David Benjamin <[email protected]>
https://boringssl-review.googlesource.com/c/boringssl/+/66288 allowed C++ runtime in libssl. The build script of bssl-sys crate should indicate that the crate requires a C++ runtime. Use libc++ on MacOS and libstdc++ on other unix like systems by default. Introduce a new environment variable to configure C++ runtime to use. Change-Id: Ib445955012126080dd03ad7b650287ea9dde10b0 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67147 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]>
Change-Id: I569110a8648f0504897b9ea94b115cd038149ace Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67327 Auto-Submit: David Benjamin <[email protected]> Commit-Queue: Bob Beck <[email protected]> Reviewed-by: Bob Beck <[email protected]>
Change-Id: Iaa166136b4b7700e59c3a7643ec1b4aacf43c647 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66747 Auto-Submit: Adam Langley <[email protected]> Reviewed-by: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]>
Combined with https://crrev.com/c/5346536, this should, I believe, avoid needing to re-download the toolchains over and over on every CI run once the caches all fill. Change-Id: I4991cf61dd164d7d39da91184ba7051ac59ce3f1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67347 Auto-Submit: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
Matching the various pre-generated builds, the CMake build no longer actually requires Go. The only things that need it are: - Running tests - Builds with -DFIPS=1 - Builds with the experimental symbol prefixing thing Bug: 542 Change-Id: I52bb427f54dd6e5719cfe77773e87fc394410380 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67367 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]>
See https://developer.apple.com/documentation/bundleresources/privacy_manifest_files Change-Id: I12e485ce294ead6a6cc16018e7e2adbb1efeddec Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67487 Auto-Submit: Adam Langley <[email protected]> Reviewed-by: David Benjamin <[email protected]> Commit-Queue: Adam Langley <[email protected]>
We're using it in parts of EVP already and this is much more readable. Change-Id: I42f30b83331cafdabd4f5d995b61176458e906bc Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67567 Auto-Submit: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]>
Some CLs crossed in-flight. Change-Id: Id98cfb7c5af67275b99df627534a633f9ae3d14a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67587 Reviewed-by: Adam Langley <[email protected]> Auto-Submit: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]> Commit-Queue: Adam Langley <[email protected]>
Noncharacters are weird. They're code points and generally expected to pass through string APIs and such, but they're also not meant to be used for "open interchange". We reject them, while most Unicode APIs accept them. They're public API nowadays, so document this. Change-Id: I56aa436ae954b591d9a00b6560617e1ad5c26d95 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67568 Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]>
The docs still describe the old implementation, but our PRNG has changed drastically since then. Change-Id: I51c34833a364a1d6bd70cf5d3b6cfb87b4aa06e7 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67569 Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> Auto-Submit: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]>
Change-Id: Ia6fffb4c1fbe9edc62a4c22b45408e41ac6ae086 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67547 Reviewed-by: David Benjamin <[email protected]> Auto-Submit: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]>
…IMPL.` Change-Id: I8340cf9259de72792a01049ecc36997233887006 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67607 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: David Benjamin <[email protected]>
After getting further with 672efb1 it turns out that there are more SVE2 instruction forms that we need to be able to recognise. Change-Id: Ia180c12ecf702e2c5536adbe1c30ac5ebd43fe75 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65727 Reviewed-by: David Benjamin <[email protected]> Commit-Queue: Adam Langley <[email protected]> Auto-Submit: Adam Langley <[email protected]>
Bug: chromium:40925630 Change-Id: Ide72960600747f5ce9a9213a9103510fee3e3806 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67527 Reviewed-by: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]>
This makes me sad, but strdup may be more trouble than is worth it? Being not in C (until C23) and only a (by POSIX standards) recent addition to POSIX means a lot of folks seem to make it unnecessarily hard to use: - MSVC adds a deprecation warning that we have to suppress - glibc gates it on feature macros; we just don't notice because we already have to work around their bad behavior for pthread_rwlock - musl gates it on feature macros, which was one of the things that tripped cl/583161936 Given we only want to use strdup in one file (err.c, which wants to avoid OPENSSL_malloc), a small reimplementation is probably not the end of the world. While I'm here, we can actually make OPENSSL_strdup's implementation a little simpler. Change-Id: I4e6c743b3104a67357d7d527c178c615de6bc844 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64047 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
These NULL checks are redundant with sk_num()'s semantics. Change-Id: I9871bd97c3188fa67f8701ba3eb12395d955d162 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67388 Commit-Queue: Adam Langley <[email protected]> Auto-Submit: Theo Buehler <[email protected]> Reviewed-by: Adam Langley <[email protected]> Reviewed-by: David Benjamin <[email protected]>
Zephyr RTOS is not a ChromeOS project, so it shouldn't be prefixed with "CROS". Actually, Zephyr build system defines "__ZEPHYR__" to tell third-party libraries that the code is compiled for Zephyr. BUG=b:321092852 Change-Id: Iba0a4c80607d6246ce523b9f92477ef3fc0bf47a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67627 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]> Reviewed-by: David Benjamin <[email protected]>
Zephyr RTOS supports multithreading with preemptive scheduling, so we must enable support for threads in BoringSSL BUG=b:321092852 Change-Id: I6fadce72e71ac043513fda0a5639261ba396b7f5 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67628 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> Reviewed-by: David Benjamin <[email protected]>
Zephyr and CrOS EC targets embedded devices without MMU unit (no virtual memory). It means that they don't support any address space duplication like fork() or clone(). BUG=b/321092852 Change-Id: Icdf8be888ba87cd164cffb35f1accbc14f1a6887 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67807 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: Bob Beck <[email protected]>
Change-Id: I881c69040a71cbe82a90e8a0074af1fdad6d046a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67827 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]> Auto-Submit: David Benjamin <[email protected]>
We actually don't test this at all right now. Change-Id: Iaac8850da3c012cbd21d0f38b026e7ff14db3650 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67828 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
Change-Id: Ie06b109fe3445129adf5eca00bd813d9957dc606 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67647 Commit-Queue: Bob Beck <[email protected]> Reviewed-by: David Benjamin <[email protected]> Auto-Submit: Bob Beck <[email protected]>
We encode the secrets in hex. When we do so, we should not leak them based on memory access patterns. Of course, the caller is presumably going to leak them anyway, because this is the SSLKEYLOGFILE callback. But it's plausible that the caller might have registered the callback unconditionally and then, in the callback, decide whether to discard the data. In that case, we should not introduce a side channel. Change-Id: If6358a3081c658038232b4610603967cb38659b4 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67829 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
do STRICT requirements on the leaf certificate, and not STRICT on the rest of the chain. Bug: 721 Change-Id: Ieec5940c0ab40aa7ea9e8fe192e5734326b976c3 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67787 Reviewed-by: David Benjamin <[email protected]> Auto-Submit: Bob Beck <[email protected]> Commit-Queue: Bob Beck <[email protected]>
To a definitely not real one. Otherwise tests fail if you do actually support DHKEM(P-256, HKDF-SHA256). Change-Id: Icb60458bba94e8df1954f154f5f6c9965543af45 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67927 Reviewed-by: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]> Commit-Queue: Roland Shoemaker <[email protected]>
Add certificates to a couple of tests which expect a specific host name, but were using the default certificate which had a different host name. Change-Id: I03e3be21e1e47313e5dbd5b8bba937b6968533c6 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67947 Commit-Queue: Roland Shoemaker <[email protected]> Reviewed-by: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]> Auto-Submit: Roland Shoemaker <[email protected]>
Change-Id: I6bd3973bfabd99b4ef1725c761a994a9e8cd5cac Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67889 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: Matt Mueller <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.