feat(connlib): utilise GSO for UDP sockets

Context
At present, connlib sends UDP packets one at a time. Sending a packet requires us to make a syscall which is quite expensive. Under load, i.e. during a speedtest, syscalls account for over 50% of our CPU time 0. In order to improve this situation, we need to somehow make use of GSO (generic segmentation offload). With GSO, we can send multiple packets to the same destination in a single syscall.
The tricky question here is, how can we achieve having multiple UDP packets ready at once so we can send them in a single syscall? Our TUN interface only feeds us packets one at a time and connlib's state machine is single-threaded. Additionally, we currently only have a single EncryptBuffer in which the to-be-sent datagram sits.
1. stack-allocating encrypted IP packets
As a first step, we get rid of the single EncryptBuffer and instead stack-allocate each encrypted IP packet. Due to our small MTU, these packets are only around 1300 bytes. Stack-allocating that requires a few memcpy's but those are in the single-digit % range in the terms of CPU time performance hit. That is nothing compared to how much time we are spending on UDP syscalls. With the EncryptBuffer out the way, we can now "freely" move around the EncryptedPacket structs and - technically - we can have multiple of them at the same time.
2. Implementing GSO
The GSO interface allows you to pass multiple packets of the same length and for the same destination in a single syscall, meaning we cannot just batch-up arbitrary UDP packets. Counterintuitively, making use of GSO requires us to do more copying: In particular, we change the interface of Io such that "sending" a packet performs essentially a lookup of a BytesMut-buffer by destination and packet length and appends the payload to that packet.
3. Be smart when to flush UDP sockets
We need to work out, when to flush these buffers. If we were to flush them at every Io::poll, then we would never actually perform GSO because we loop back to Io::poll after sending each packet. However, if we don't flush them at the top of Io::poll, we risk continuously growing memory if the TUN interface is faster in sending us packets than we can send them out over the UDP socket. The priorities within Io::poll should always follow the mantra of: Keep buffers small and finish local work before accepting new work (read: packets from sockets). Thus, the order in which we do things basically represents these priorities.
The solution here is to flush the buffers at the end of Io::poll AND in case the buffer grows > 1MB. Flushing at the end of Io::poll means we can handle bursts of packets from the TUN device, encapsulate them and then send them out in a single syscall.
4. Batch-read IP packets
Finally, in order to further improve the effectiveness of GSO, we batch-read up to 50 IP packets from the mpsc-channel that connects connlib's main event-loop with the dedicated thread that reads and writes to the TUN device. These reads and writes happen concurrently to connlib's packet processing. Thus, it is likely that by the time connlib is ready to process another IP packet, multiple have been read from the device and are sitting in the channel. Batch-processing these IP packets means that the buffers in our GsoQueue are more likely to contain more than a single datagram.
Imagine you are running a file upload. The OS will send many packets to the same destination IP and likely max MTU to the TUN device. It is likely, that we read 10-20 of these packets in one batch (i.e. within a single "tick" of the event-loop). All packets will be appended to the same buffer in the GsoQueue and on the next event-loop tick, they will all be flushed out in a single syscall.
Results
Overall, this results in a significant reduction of syscalls for sending UDP message. In 1, we spend only a total of 16% of our CPU time in udpv6_sendmsg whereas in 0 (main), we spent a total of 34%. Do note that these numbers are relative to the total CPU time spent per program run and thus can't be compared directly (i.e. you cannot just do 34 - 16 and say we now spend 18% less time sending UDP packets). Nevertheless, this appears to be a great improvement.
In terms of throughput, we achieve a ~60% improvement in our benchmark suite. That one is running on localhost though so it might not necessarily be reflect like that in a real network.

Still need to test this on other platforms but looks promising :)
I am especially curious on how what impact this will have on the gateway.

Still need to test this on other platforms but looks promising :)
I am especially curious on how what impact this will have on the gateway.

Just to make sure I'm following, what other platforms would GSO be applicable to? I thought this was a Linux-only API?
Yeah the Gateway is the main thing that will benefit from this for sure.

Just to make sure I'm following, what other platforms would GSO be applicable to? I thought this was a Linux-only API?

As of recently, quinn-udp also has support for GSO on Apple: https://github.com/quinn-rs/quinn/pull/1993.

Do note that these numbers are relative to the total CPU time spent per program run and thus can't be compared directly (i.e. you cannot just do 34 - 21 and say we now spend 13% less time sending UDP packets).

@conectado You are better at statistics than me, am I reading thinking through this correctly? :)

Not quite working on Apple it seems :D
https://firezone-inc.sentry.io/issues/6037207250

Curious where a bit more time profiling and optimization can get us!

The next thing will be creating multiple threads for the TUN device and reading / writing those in parallel.

@jamilbk If you have the time, I'd be interested to learn if you get any better upload speed on your Mac with this. I only have 40MBit upload here and that pretty much tops out already anyway.

It would be nice to be able to have benchmarks / blog post to point users to on sales calls. Sizing is one of the main questions / friction points admins have when deploying Gateways.

https://github.com/firezone/firezone/issues/7243
On top of that, we could easily build something where we run benchmarks on differently sized machines to see how that affects the throughput.

Do note that these numbers are relative to the total CPU time spent per program run and thus can't be compared directly (i.e. you cannot just do 34 - 21 and say we now spend 13% less time sending UDP packets).

@conectado You are better at statistics than me, am I thinking through this correctly? :)

I think you're correct! those number are relative to CPU time and if we consider we measured the program for long enough to average out any outlier event and we assume the same conditions, it should actually represent 13% less CPU time spent on udpv6_sendmsg, which probably means it was called less times as there is no reason to assume an speed up on that function itself.
But as you said, we can't assume it means we spent 13% less time on processing a packet as that time could be diverted to any other stack-frame, to actually be sure of the time improvements we should see the whole packet processing in a single stack-frame.
Which we might be actually be able to do if we make some benchmarks with a custom scheduler? It would be an interesting avenue to explore if we want to continue making performance improvements.

Wow! The performance improvement of this in our benchmark suite is insane. The throughput for TCP download jumped from 380 MBit to 620 MBit!
https://bencher.dev/console/projects/firezone-1l75jv1z/perf?lower_value=false&upper_value=false&lower_boundary=false&upper_boundary=false&x_axis=date_time&branches=455d0566-7315-404c-93e1-877e2732cc97%2Ccaf219c2-01aa-4d89-856a-4daf49097daa&testbeds=258ac4fc-9cc9-4281-a45b-666cd8264371&benchmarks=f0008569-a7be-4f17-8f53-37abe7cbc31b%2Cb35d8c57-795d-4eac-9105-6911d81320a7%2C0747e62d-e0e0-4fa0-a5af-709dc13f5e81%2Cafea6d44-89ac-4e9f-ab0e-8b6d2573e9c7%2Cdbebadcd-b0cf-4abd-a4f8-7274f064f6da%2C0c0d7024-2671-4c01-a775-d6261677d23f%2Cf01099cd-c22e-4a6e-b2ac-5f8f1d4e82c9%2Ccf34713e-228b-4609-97ea-d6404d243933&measures=a777fd3c-5f6c-4884-82e8-39cd8b2d30f1&start_time=1731542400000&end_time=1731643214415&tab=branches&plots_search=b833fb76-2308-4103-baa4-99d05ac8c819&key=true&reports_per_page=4&branches_per_page=8&testbeds_per_page=8&benchmarks_per_page=8&plots_per_page=8&reports_page=1&branches_page=1&testbeds_page=1&benchmarks_page=1&plots_page=1&branches_search=gso&clear=true&heads=%2C93fe2e4b-22ca-43ed-94ab-691a2f41117a

I tested this on Linux, Windows (VM) and MacOS and couldn't find any problems! Unfortunately, me Internet connection (100 / 40) is too slow to notice an improvement as we can already saturate that on main too. Thus, my suggestion would be to merge that so that it gets deployed to the AWS gateway and we measure its impact with e.g. concurrent speed tests and from places with a faster Internet connection.

@jamilbk I am tidying up the commits a bit to make them easier to review.

We need to work out, when to flush these buffers. If we were to flush them at every Io::poll, then we would never actually perform GSO because we loop back to Io::poll after sending each packet. However, if we don't flush them at the top of Io::poll, we risk continuously growing memory if the TUN interface is faster in sending us packets than we can send them out over the UDP socket. The priorities within Io::poll should always follow the mantra of: Keep buffers small and finish local work before accepting new work (read: packets from sockets). Thus, the order in which we do things basically represents these priorities.
The solution here is to flush the buffers at the end of Io::poll AND in case the buffer grows > 1MB. Flushing at the end of Io::poll means we can handle bursts of packets from the TUN device, encapsulate them and then send them out in a single syscall.

I think with the new idea of batch-reading IP packets, we no longer need this which makes things a bit simpler!

Appears to be broken on Android: https://github.com/quinn-rs/quinn/issues/2042

Testing

[ ] Linux
[ ] Windows
[ ] MacOS
[ ] Android
[ ] iOS

Testing (commit 052ac18)
* [x]  Linux

* [x]  Windows

* [x]  MacOS

* [x]  Android

* [ ]  iOS

@jamilbk Do you have an iOS device that you could test this on? All platforms are working now! :tada:

iOS works fine.
Seeing packet loss for the Windows build from this branch:

Tests seemed ok on iOS and Windows. Not much if any speed difference.

Compared to a build on main? I haven't done any benchmarking on Windows so I am not surprised if the bottleneck is somewhere else there.


Error while creating SpringBoot CDS image in Bitbucket Pipelines

Expected Behavior
Everything works and the Docker image is built using CDS
Current Behavior
[INFO]     [creator]     ===> DETECTING
[INFO]     [creator]     target distro name/version labels not found, reading /etc/os-release file
[INFO]     [creator]     target distro name/version labels not found, reading /etc/os-release file
[INFO]     [creator]     4 of 7 buildpacks participating
[INFO]     [creator]     bellsoft/buildpacks/liberica     1.1.0
[INFO]     [creator]     paketo-buildpacks/syft           1.47.0
[INFO]     [creator]     paketo-buildpacks/executable-jar 6.10.0
[INFO]     [creator]     paketo-buildpacks/spring-boot    5.30.0
[INFO]     [creator]     ===> RESTORING
[INFO]     [creator]     ===> BUILDING
[INFO]     [creator]     target distro name/version labels not found, reading /etc/os-release file
[INFO]     [creator]
[INFO]     [creator]     Paketo Buildpack for BellSoft Liberica JDK Lite and Native Image Kit 1.1.0
[INFO]     [creator]       https://github.com/bell-sw/buildpacks/liberica
[INFO]     [creator]       Build Configuration:
[INFO]     [creator]         $BP_JVM_JLINK_ARGS           --no-man-pages --no-header-files --strip-debug --compress=1  configure custom link arguments (--output must be omitted)
[INFO]     [creator]         $BP_JVM_JLINK_ENABLED        false                                                        enables running jlink tool to generate custom JRE
[INFO]     [creator]         $BP_JVM_TYPE                 JRE                                                          the JVM type - JDK or JRE
[INFO]     [creator]         $BP_JVM_VERSION              21                                                           the Java version
[INFO]     [creator]       Launch Configuration:
[INFO]     [creator]         $BPL_DEBUG_ENABLED           false                                                        enables Java remote debugging support
[INFO]     [creator]         $BPL_DEBUG_PORT              8000                                                         configure the remote debugging port
[INFO]     [creator]         $BPL_DEBUG_SUSPEND           false                                                        configure whether to suspend execution until a debugger has attached
[INFO]     [creator]         $BPL_HEAP_DUMP_PATH                                                                       write heap dumps on error to this path
[INFO]     [creator]         $BPL_JAVA_NMT_ENABLED        true                                                         enables Java Native Memory Tracking (NMT)
[INFO]     [creator]         $BPL_JAVA_NMT_LEVEL          summary                                                      configure level of NMT, summary or detail
[INFO]     [creator]         $BPL_JFR_ARGS                                                                             configure custom Java Flight Recording (JFR) arguments
[INFO]     [creator]         $BPL_JFR_ENABLED             false                                                        enables Java Flight Recording (JFR)
[INFO]     [creator]         $BPL_JMX_ENABLED             false                                                        enables Java Management Extensions (JMX)
[INFO]     [creator]         $BPL_JMX_PORT                5000                                                         configure the JMX port
[INFO]     [creator]         $BPL_JVM_HEAD_ROOM           0                                                            the headroom in memory calculation
[INFO]     [creator]         $BPL_JVM_LOADED_CLASS_COUNT  35% of classes                                               the number of loaded classes in memory calculation
[INFO]     [creator]         $BPL_JVM_THREAD_COUNT        250                                                          the number of threads in memory calculation
[INFO]     [creator]         $JAVA_TOOL_OPTIONS                                                                        the JVM launch flags
[INFO]     [creator]         Using Java version 21 from BP_JVM_VERSION
[INFO]     [creator]       BellSoft Liberica JRE (musl) 21.0.3: Contributing to layer
[INFO]     [creator]         Downloading from https://packages.bell-sw.com/alpaquita/java/21.0.3+10/bellsoft-jre21.0.3+10-linux-x64-musl-lite.tar.gz
[INFO]     [creator]         Verifying checksum
[INFO]     [creator]         Expanding to /layers/bellsoft_buildpacks_liberica/jre
[INFO]     [creator]         Adding 147 container CA certificates to JVM truststore
[INFO]     [creator]         Writing env.build/JAVA_HOME.default
[INFO]     [creator]         Writing env.build/JRE_HOME.default
[INFO]     [creator]         Writing env.launch/BPI_APPLICATION_PATH.default
[INFO]     [creator]         Writing env.launch/BPI_JVM_CACERTS.default
[INFO]     [creator]         Writing env.launch/BPI_JVM_CLASS_COUNT.default
[INFO]     [creator]         Writing env.launch/BPI_JVM_SECURITY_PROVIDERS.default
[INFO]     [creator]         Writing env.launch/JAVA_HOME.default
[INFO]     [creator]         Writing env.launch/JAVA_TOOL_OPTIONS.append
[INFO]     [creator]         Writing env.launch/JAVA_TOOL_OPTIONS.delim
[INFO]     [creator]         Writing env.launch/MALLOC_ARENA_MAX.default
[INFO]     [creator]       Launch Helper: Contributing to layer
[INFO]     [creator]         Creating /layers/bellsoft_buildpacks_liberica/helper/exec.d/java-opts
[INFO]     [creator]         Creating /layers/bellsoft_buildpacks_liberica/helper/exec.d/jvm-heap
[INFO]     [creator]         Creating /layers/bellsoft_buildpacks_liberica/helper/exec.d/link-local-dns
[INFO]     [creator]         Creating /layers/bellsoft_buildpacks_liberica/helper/exec.d/memory-calculator
[INFO]     [creator]         Creating /layers/bellsoft_buildpacks_liberica/helper/exec.d/security-providers-configurer
[INFO]     [creator]         Creating /layers/bellsoft_buildpacks_liberica/helper/exec.d/jmx
[INFO]     [creator]         Creating /layers/bellsoft_buildpacks_liberica/helper/exec.d/jfr
[INFO]     [creator]         Creating /layers/bellsoft_buildpacks_liberica/helper/exec.d/openssl-certificate-loader
[INFO]     [creator]         Creating /layers/bellsoft_buildpacks_liberica/helper/exec.d/security-providers-classpath-9
[INFO]     [creator]         Creating /layers/bellsoft_buildpacks_liberica/helper/exec.d/debug-9
[INFO]     [creator]         Creating /layers/bellsoft_buildpacks_liberica/helper/exec.d/nmt
[INFO]     [creator]       Java Security Properties: Contributing to layer
[INFO]     [creator]         Writing env.launch/JAVA_SECURITY_PROPERTIES.default
[INFO]     [creator]         Writing env.launch/JAVA_TOOL_OPTIONS.append
[INFO]     [creator]         Writing env.launch/JAVA_TOOL_OPTIONS.delim
[INFO]     [creator]
[INFO]     [creator]     Paketo Buildpack for Syft 1.47.0
[INFO]     [creator]       https://github.com/paketo-buildpacks/syft
[INFO]     [creator]         Downloading from https://github.com/anchore/syft/releases/download/v0.105.1/syft_0.105.1_linux_amd64.tar.gz
[INFO]     [creator]         Verifying checksum
[INFO]     [creator]         Writing env.build/SYFT_CHECK_FOR_APP_UPDATE.default
[INFO]     [creator]
[INFO]     [creator]     Paketo Buildpack for Executable JAR 6.10.0
[INFO]     [creator]       https://github.com/paketo-buildpacks/executable-jar
[INFO]     [creator]       Command "packages" is deprecated, use `syft scan` instead
[INFO]     [creator]       Class Path: Contributing to layer
[INFO]     [creator]         Writing env/CLASSPATH.delim
[INFO]     [creator]         Writing env/CLASSPATH.prepend
[INFO]     [creator]       Process types:
[INFO]     [creator]         executable-jar: java org.springframework.boot.loader.launch.JarLauncher (direct)
[INFO]     [creator]         task:           java org.springframework.boot.loader.launch.JarLauncher (direct)
[INFO]     [creator]         web:            java org.springframework.boot.loader.launch.JarLauncher (direct)
[INFO]     [creator]
[INFO]     [creator]     Paketo Buildpack for Spring Boot 5.30.0
[INFO]     [creator]       https://github.com/paketo-buildpacks/spring-boot
[INFO]     [creator]       Build Configuration:
[INFO]     [creator]         $BPL_JVM_CDS_ENABLED                 false  whether to enable CDS optimizations at runtime
[INFO]     [creator]         $BPL_SPRING_AOT_ENABLED              false  whether to enable Spring AOT at runtime
[INFO]     [creator]         $BP_JVM_CDS_ENABLED                  true   whether to enable CDS & perform JVM training run
[INFO]     [creator]         $BP_SPRING_AOT_ENABLED               true   whether to enable Spring AOT
[INFO]     [creator]         $BP_SPRING_CLOUD_BINDINGS_DISABLED   false  whether to contribute Spring Boot cloud bindings support
[INFO]     [creator]         $BP_SPRING_CLOUD_BINDINGS_VERSION    1      default version of Spring Cloud Bindings library to contribute
[INFO]     [creator]       Launch Configuration:
[INFO]     [creator]         $BPL_SPRING_CLOUD_BINDINGS_DISABLED  false  whether to auto-configure Spring Boot environment properties from bindings
[INFO]     [creator]         $BPL_SPRING_CLOUD_BINDINGS_ENABLED   true   Deprecated - whether to auto-configure Spring Boot environment properties from bindings
[INFO]     [creator]       Spring Cloud Bindings 2.0.3: Contributing to layer
[INFO]     [creator]         Downloading from https://repo1.maven.org/maven2/org/springframework/cloud/spring-cloud-bindings/2.0.3/spring-cloud-bindings-2.0.3.jar
[INFO]     [creator]         Verifying checksum
[INFO]     [creator]         Copying to /layers/paketo-buildpacks_spring-boot/spring-cloud-bindings
[INFO]     [creator]       Performance: Contributing to layer
[INFO]     [creator]         Extracting Jar
[INFO]     [creator]     unable to invoke layer creator
[INFO]     [creator]     unable to contribute spring-cds layer
[INFO]     [creator]     error resetting file times
[INFO]     [creator]     chtimes .: operation not permitted
[INFO]     [creator]     ERROR: failed to build: exit status 1
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  47.682 s
[INFO] Finished at: 2024-07-18T12:03:37Z
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.springframework.boot:spring-boot-maven-plugin:3.3.1:build-image (default-cli) on project oikos-api: Execution default-cli of goal org.springframework.boot:spring-boot-maven-plugin:3.3.1:build-image failed: Builder lifecycle 'creator' failed with status code 51 -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/PluginExecutionException

Possible Solution
Document if a Bitbucket runner image is not supported and if there is a workaround
Steps to Reproduce

Generate a Spring Boot project
Use the build-image spring boot plugin with a configuration using CDS

Motivations
I want to give developers the availability to create different kinds of Docker images preconfigured in the company parent pom (Standard, Native, CDS and Crac if it's supported in the future).
Standard and native work, but CDS gives me this problem
Additional
I have already implemented another workarounds to make the build-image work in Bitbucket Pipelines

Setting DOCKER_HOST=tcp://<IP_ADDRESS>:2375 just before executing the plugin
Setting this Maven configs

<configuration>
          <docker>
            <bindHostToBuilder>true</bindHostToBuilder>
          </docker>
          <image>
            <builder>paketobuildpacks/builder-jammy-tiny:latest</builder>
            <env>
              <BP_JVM_VERSION>${java.version}</BP_JVM_VERSION>
            </env>
            <securityOptions/>
            <buildWorkspace>
              <bind>
                <source>/opt/atlassian/bitbucketci/agent/build/cache-${project.artifactId}.work
                </source>
              </bind>
            </buildWorkspace>
            <buildCache>
              <bind>
                <source>/opt/atlassian/bitbucketci/agent/build/cache-${project.artifactId}.build
                </source>
              </bind>
            </buildCache>
            <launchCache>
              <bind>
                <source>/opt/atlassian/bitbucketci/agent/build/cache-${project.artifactId}.launch
                </source>
              </bind>
            </launchCache>
          </image>
          <excludes>
            <exclude>
              <groupId>org.projectlombok</groupId>
              <artifactId>lombok</artifactId>
            </exclude>
          </excludes>
        </configuration>

well, you have pretty much answered your question: for CDS, it's not an option to reset all files used during the training run to the same epoch that the layer is generated on; in this case, 1980-1-1.
So that means the buildpack needs write access to the workspace at buildtime...
If there's a way to relax the permissions in the build container of your CI, I guess it would solve your issue; we have testimonies of people using this new feature successfully in other CI systems (at least we tested with Github Actions during development)

Investigating I found this issue related to the permissions of the workspace directory, seems that Bitbucket restricts the access by default
https://github.com/buildpacks/community/discussions/229
I'll try if just changing the folder permissions works in my case

can we close this issue?
cc @TMRGZ

I couldn't get it to work, if anything changes I'll leave a comment


Fixes for docker changes plus others

Description
Docker file fixes
wkhtmltopdf swap to calibre
Blank page detection using plain java instead of python
Stamp page selection (needs webUI)
PDF to txt readd
TODO before release
stamp page UI
html test
game release fix
Merging early to fix alpha branch
Closes #(issue_number)
Checklist:

[x ] I have read the Contribution Guidelines
[ x] I have performed a self-review of my own code
[ x] I have commented my code, particularly in hard-to-understand areas
[ x] My changes generate no new warnings

Contributor License Agreement
By submitting this pull request, I acknowledge and agree that my contributions will be included in Stirling-PDF and that they can be relicensed in the future under the MPL 2.0 (Mozilla Public License Version 2.0) license.
(This does not change the general open-source nature of Stirling-PDF, simply moving from one license to another license)

@CodiumAI-Agent /improve

@CodiumAI-Agent /describe

Generating PR code suggestions
Work in progress ...

@CodiumAI-Agent /review

PR Code Suggestions
💡 [possible issue]
It seems that the isBlankImage method is not handling the case when the image is null. This could lead to a NullPointerException if the image is null. It would be better to handle this case explicitly.
File: src/main/java/stirling/software/SPDF/controller/api/misc/BlankPageController.java (116-120)
  Example code:

Existing code:
public static boolean isBlankImage(
    BufferedImage image, int threshold, double whitePercent, int blurSize) {
    if (image == null) {
        logger.info("Error: Image is null");
        return false;
    }
    ...
}

Improved code:
public static boolean isBlankImage(
    BufferedImage image, int threshold, double whitePercent, int blurSize) {
    if (image == null) {
        logger.info("Error: Image is null");
        throw new IllegalArgumentException("Image cannot be null");
    }
    ...
}

💡 [possible issue]
The addStamp method is not handling the case when the watermarkType is neither "text" nor "image". It would be better to handle this case explicitly to avoid unexpected behavior.
File: src/main/java/stirling/software/SPDF/controller/api/misc/StampController.java (108-134)
  Example code:

Existing code:
if ("text".equalsIgnoreCase(watermarkType)) {
    ...
} else if ("image".equalsIgnoreCase(watermarkType)) {
    ...
}

Improved code:
if ("text".equalsIgnoreCase(watermarkType)) {
    ...
} else if ("image".equalsIgnoreCase(watermarkType)) {
    ...
} else {
    throw new IllegalArgumentException("Invalid watermark type: " + watermarkType);
}

💡 [possible issue]
The processPdfToRTForTXT method is not handling the case when the outputFormat is neither "txt" nor "rtf". It would be better to handle this case explicitly to avoid unexpected behavior.
File: src/main/java/stirling/software/SPDF/controller/api/converters/ConvertPDFToOffice.java (68-82)
  Example code:

Existing code:
if ("txt".equals(request.getOutputFormat())) {
    ...
} else {
    ...
}

Improved code:
if ("txt".equals(request.getOutputFormat())) {
    ...
} else if ("rtf".equals(request.getOutputFormat())) {
    ...
} else {
    throw new IllegalArgumentException("Invalid output format: " + request.getOutputFormat());
}

💡 [possible issue]
The runInstallCommandBasedOnEnvironment method is not handling the case when the bookAndHtmlFormatsInstalled is false. It would be better to handle this case explicitly to avoid unexpected behavior.
File: src/main/java/stirling/software/SPDF/config/PostStartupProcesses.java (38-43)
  Example code:

Existing code:
if (bookAndHtmlFormatsInstalled) {
    ...
}

Improved code:
if (bookAndHtmlFormatsInstalled) {
    ...
} else {
    logger.info("Book and HTML formats are not installed");
}

PR Analysis

🎯 Main theme: Refactoring and bug fixes
📝 PR summary: This PR includes several changes, mainly focusing on refactoring and bug fixes. The changes include updates to Docker files, swapping wkhtmltopdf to calibre, implementing blank page detection using Java instead of Python, and adding stamp page selection. The PR also includes a few other minor changes and fixes.
📌 Type of PR: Bug fix
🧪 Relevant tests added: No
⏱ Estimated effort to review [1-5]: 4, because the PR contains a large number of changes across multiple files, including logic changes and refactoring. It requires a good understanding of the project and its dependencies to review effectively.
🔒 Security concerns: No security concerns found

PR Feedback
💡 General suggestions: The PR includes a lot of changes, which makes it harder to review. It would be better to split these changes into smaller, more manageable PRs. Also, it's important to add tests for the changes made, especially for the logic changes in the BlankPageController and FileToPdf classes.
 🤖 Code feedback:relevant filesrc/main/java/stirling/software/SPDF/controller/api/misc/BlankPageController.javasuggestion
Consider using a more efficient way to calculate the white pixel percentage. The current method involves iterating over every pixel in the image, which can be slow for large images. [medium]
relevant line'+                for (int i = 0; i < image.getHeight(); i++) {'relevant filesrc/main/java/stirling/software/SPDF/controller/api/misc/StampController.javasuggestion
It would be better to validate the page numbers provided by the user. Currently, the code assumes that the user will provide valid page numbers. [important]
relevant line'+        for (int pageIndex : pageNumbers) {'relevant filesrc/main/java/stirling/software/SPDF/controller/api/converters/ConvertPDFToOffice.javasuggestion
Instead of hardcoding the MediaType.TEXT_PLAIN, consider determining the MediaType based on the output format. This would make the code more flexible if other output formats are added in the future. [medium]
relevant line'+                        MediaType.TEXT_PLAIN);'relevant filesrc/main/java/stirling/software/SPDF/controller/api/converters/ConvertHtmlToPDF.javasuggestion
The method convertHtmlToPdf is called with five arguments, which makes it harder to understand. Consider using a DTO (Data Transfer Object) to encapsulate these parameters. [medium]
relevant line'+                FileToPdf.convertHtmlToPdf('

 ✨ Usage guide:
Overview:
The review tool scans the PR code changes, and generates a PR review. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.
When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

With a configuration file, use the following template:
[pr_reviewer]
some_config1=...
some_config2=...

  Utilizing extra instructions
The review tool can be configured with extra instructions, which can be used to guide the model to a feedback tailored to the needs of your project.
Be specific, clear, and concise in the instructions. With extra instructions, you are the prompter. Specify the relevant sub-tool, and the relevant aspects of the PR that you want to emphasize.
Examples for extra instructions:
[pr_reviewer] # /review #
extra_instructions="""
In the code feedback section, emphasize the following:
- Does the code logic cover relevant edge cases?
- Is the code logic clear and easy to understand?
- Is the code logic efficient?
...
"""

Use triple quotes to write multi-line instructions. Use bullet points to make the instructions more readable.

  How to enable\disable automation

When you first install PR-Agent app, the default mode for the review tool is:

pr_commands = ["/review", ...]

meaning the review tool will run automatically on every PR, with the default configuration.
Edit this field to enable/disable the tool, or to change the used configurations

  About the 'Code feedback' section
The review tool provides several type of feedbacks, one of them is code suggestions.
If you are interested only in the code suggestions, it is recommended to use the improve feature instead, since it dedicated only to code suggestions, and usually gives better results.
Use the review tool if you want to get a more comprehensive feedback, which includes code suggestions as well.

  Auto-labels
The review tool can auto-generate two specific types of labels for a PR:

a possible security issue label, that detects possible security issues (enable_review_labels_security flag)
a Review effort [1-5]: x label, where x is the estimated effort to review the PR (enable_review_labels_effort flag)

  Extra sub-tools
The review tool provides a collection of possible feedbacks about a PR.
It is recommended to review the possible options, and choose the ones relevant for your use case.
Some of the feature that are disabled by default are quite useful, and should be considered for enabling. For example:
require_score_review, require_soc2_review, enable_review_labels_effort, and more.

  More PR-Agent commands

To invoke the PR-Agent, add a comment using one of the following commands:

/review: Request a review of your Pull Request.
/describe: Update the PR title and description based on the contents of the PR.
/improve [--extended]: Suggest code improvements. Extended mode provides a higher quality feedback.
/ask <QUESTION>: Ask a question about the PR.
/update_changelog: Update the changelog based on the PR's contents.
/add_docs 💎: Generate docstring for new components introduced in the PR.
/generate_labels 💎: Generate labels for the PR based on the PR's contents.
/analyze 💎: Automatically analyzes the PR, and presents changes walkthrough for each component.

See the tools guide for more details.
To list the possible configuration parameters, add a /config comment.

See the review usage page for a comprehensive guide on using this tool.


[Revamp] Add memory limits

Add memory limits configuration to world.in
Memory usage could be calculated as:
Working memory
$\text{arraySize} + \text{parameters} + \text{No. parameters in current function} + \text{No. Variables in current scope} $
Total memory
$\text{arraySize} + \text{StackUsage} $

Needed when array is available, in the meantime, the stack size is enough.


kubeconfig, podCidr support

Support kubeconfig as means of accessing / authenticating with API server.
Read podCidr from k8s API when using host-local IPAM with subnet: usePodCidr

This would be much easier to review if 1) it was two separate PRs 2) You trimmed the space off lines in a different PR still.

Yeah, sorry about those lines... I didn't really notice it was happening :(

https://github.com/projectcalico/calico-cni/blob/master/configuration.md needs updating as part of this PR

Looks good. I'm a bit suspicious of the caching, we should talk some more about that.

OK - I've removed the caching and responded to comments.  Also brought test coverage back up to 100%.

A few more small comments. Once resolved I'm happy for you to merge. LGTM


Update orion.node_status to use color for alias results reflecting node stats

Orion
Status

pack extension, complete.

Description
Use the color reflecting a node status that was already in the results and use the extra color support from 1.4 in the orion.node_status alias results.
Checklist (tick everything that applies)

[X] Version bump (required for changed packs)
[ ] Code linting (required, can be done after the PR checks)
[X] Tests (not required but really recommended)

Nice! Something else you should probably do is mention in README.md that this pack is only compatible with st2 1.4+. It's very new, so for some time we should keep warning people whenever an alias utilizes the new features.

Very good suggestion, updated the README.md with something about ST2 versions.

Cool! Another suggestion is to have your action give hex values for colors, since extra.color will hopefully be cross-adapter in the future. good, danger, etc. are Slack-only, so they won't work elsewhere.

Merging, but please don't forget about the colors. :)


Improvements

We should sync updates made on the lanekeeping and GXZ_kin branches.  This would primarily be done by modifying the parse_rosbag scripts to capture the road frame and curvature information into the mat file.  And then by adjusting the messages / main controller code to handle variation of the controller used.
Also, may be a good idea to remove unneeded elements from the analysis folder and/or add a README for future improvement.

Speaking of improvements, I am curious why you switch from julia/JuMP to python/CASADI? I am also curious how the performance stacks up?
Thanks and great work!

So there were a few reasons to move away from JuMP:
(1) Initially I used RobotOS.jl but it was slow (I think it needs to compile messages every time you run a node).
(2) I had precompilation working for the MPC module in Julia but it may still need a couple seconds to start up.
(3) I tried to use pyjulia to remove the requirement of having a main script be in Julia, so only the MPC portion could be written in Julia.  This is tricky since I could only get this working smoothly with specific versions of Julia and Python.
(4) Casadi provides a similar interface to JuMP if you use the opti interface and starts almost immediately.  I think, under the hood, it's basically a wrapper to Ipopt in C++ so the speeds are pretty good.
I found that, with a warm start, Casadi/Ipopt can find solutions at 50 Hz.  Earlier, with JuMP/Ipopt, I could only get max 20 Hz, although it's likely I wasn't properly warm starting (just relied on whatever is cached when you call the solve function in JuMP).  This hasn't been tested though with difficult optimization problems (e.g. collision avoidance); in this repo, usually the warm start is close to optimal since the reference path changes slowly.
So mostly changed due to simplicity, speed of start up with ROS/Python, and the fact that I don't need an additional install/configuration of Julia.  Just need to do pip install casadi and that's it.

Thank you very much for going into the details of this.
I definitely know the struggle of setting things up with Julia/RobotOS/Python and ROS, possible, but not fun.
Wow, there is a huge difference in speed!

to stop printing all the messages, try
		p_opts = {"expand": True, "print_time":False}


Request.script_name is not set properly when ENV['RACK_BASE_URI'] is used

Starting Padrino with environment value "ENV['RACK_BASE_URI']",
% export RACK_BASE_URI=/nonroot; padrino start
then, I get request uri from Rack::request instance,

request.fullpath

but it doesn't return correct value because request.script_name is not set.
request.script_name should be set as ENV['RACK_BASE_URI'] value.
This breaks some library such as will_pagenate.

We rely 100% on Rack for this.
-- Gesendet von meinem Palm Prē
Davide D'Agostino<EMAIL_ADDRESS>schrieb am 04.02.2012 02:37:
Okey thanks for that, I think @rtomakyo or @rkh have a better idea about that. Thoughts?

Reply to this email directly or view it on GitHub:
https://github.com/padrino/padrino-framework/issues/773#issuecomment-3807418


"Get DOI now" button requires draft to be saved first

Package version (if known): 7.0.1
Describe the bug
A clear and concise description of what the bug is.
Steps to Reproduce

Go to https://<IP_ADDRESS>:5000/uploads/new
Click on 'Save draft'
Select "No" to "Do you already have a DOI for this upload?"
Click "Get DOI now!"

See that
An error is reported: "A PID already exists for type doi".
Expected behavior
A DOI should be created and no error reported
Screenshots (if applicable)

Additional context

I'm able to reproduce it in v8.

Yes, we have not yet fixed the issue


Choosing Tools>Customize Colors crashes VS 2017

Fresh VS 2017 install with the Color themes extension installed. When you go to Tools at the top, and then Color Themes, Visual Studio crashes and then restarts. Happens each time, whether you're on the Start page or a solution is open.

Can you please confirm which extension you installed?

Color Theme Editor for Visual Studio 2017
Color Themes for for Visual Studio
You can check in Tools->Extensions and Updates

The first one, with 2017 in the name.
On Oct 18, 2017 12:04 PM, "Justin Clareburt"<EMAIL_ADDRESS>wrote:
Can you please confirm which extension you installed?

Color Theme Editor for Visual Studio 2017
https://marketplace.visualstudio.com/items?itemName=VisualStudioProductTeam.VisualStudio2017ColorThemeEditor
Color Themes for for Visual Studio
https://marketplace.visualstudio.com/items?itemName=JustinClareburtMSFT.ColorThemesforVisualStudio
You can check in Tools->Extensions and Updates

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/Microsoft/VS-ColorThemes/issues/10#issuecomment-337641867,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AeABorZVFFzrSaqsSoPPEPRnVMbLhTcvks5stiGYgaJpZM4P9hpW
.

Thanks

If it matters I installed a bit of an older version of VS, version 15.2 (26430.6)

Maybe something is corrupted with your installation. I recommend you upgrade to the latest public version. (Since VS2017, upgrades have become much smaller, faster and more reliable. So it's become much easier to keep up-to-date with the latest release.)
Failing that, I would consider completely uninstalling and reinstalling Visual Studio. (Again, this should not take too long. You'll just need to download and reinstall all your favourite extensions. You might also find them in the "Roaming Extensions" list in Tools->Extensions and Updates.
Hope that helps. Please let me know if you have success.

Would love to update to the latest version, but even though I selected my
other HDD, VS still installs most of the program on my C drive, which is an
SSD and I don't have enough space! I've got around 2TB on my other HDD but
nooo, we can't install Visual Studio there, we need to install it on your C
drive, even though you have limited space.
On Oct 18, 2017 5:09 PM, "Justin Clareburt"<EMAIL_ADDRESS>wrote:
Maybe something is corrupted with your installation. I recommend you
upgrade to the latest public version. (Since VS2017, upgrades have become
much smaller, faster and more reliable. So it's become much easier to keep
up-to-date with the latest release.)
Failing that, I would consider completely uninstalling and reinstalling
Visual Studio. (Again, this should not take too long. You'll just need to
download and reinstall all your favourite extensions. You might also find
them in the "Roaming Extensions" list in Tools->Extensions and Updates.
Hope that helps. Please let me know if you have success.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/Microsoft/VS-ColorThemes/issues/10#issuecomment-337728638,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AeABouSRamPpHNtE2l9IXXfE1S0-cN5nks5stmkcgaJpZM4P9hpW
.

What's sad is VS installer tells me I need 14GB to complete the update but I don't have enough space, and to try selecting another drive. I've already done that! But apparently even though the updates have gotten much smaller, it needs 14GB on my C drive and then even more on my other HDD...

That sounds like a terrible experience.

Hi Nate
I've spoken to the team, and here's the information I've been given:

Visual Studio itself installs to the drive you select, but a lot of the payload of VS is other stuff – SDKs, emulators, runtimes – and a lot of that installs into the %ProgramFiles% folder. It’s not ideal, and there are plans to fix it but it keeps being bumped down the backlog because it’s an expensive / risky change.

There are some other possibilities available – including manually moving the ‘package cache’ to another drive.

Probably the best bet is to go to our new Gitter channel at https://gitter.im/Microsoft/VisualStudio - because that will make it easy for you to engage with us.

@NateDev100 are you able to run repair on your installation without filling up your hard drive? I just tested your scenario on the same build of VS that you have and I cannot reproduce the issue. I agree with @justcla that it is likely something corrupted with your installation.

Unfortunately not, I don’t have enough space even to repair the install, but for now I’ve just gone without the color themes until I can free up space or buy a new SSD 😊
Sent from Mail for Windows 10
From: Daniel Griffen
Sent: Thursday, October 26, 2017 7:33 PM
To: Microsoft/VS-ColorThemes
Cc: NateDev100; Mention
Subject: Re: [Microsoft/VS-ColorThemes] Choosing Tools>Customize Colorscrashes VS 2017 (#10)
@NateDev100 are you able to run repair on your installation without filling up your hard drive? I just tested your scenario on the same build of VS that you have and I cannot reproduce the issue. I agree with @justcla that it is likely something corrupted with your installation.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

This should be logged against the Color Theme Editor extension


server_error Invalid Token

Hello,
I am using this API the first time, when I am running your example file the first example shows the following error,
Error Code: server_error
Error Message: Invalid token
I have set the API_KEY variable.
i have attached a screenshot of code.

I'm having the same issues. I haven't been able to get it to accept my key either. There are a number of settings that I'm guessing at the time didn't exist. For example, is this using oauth or jwt? should we be set to application or enterprise. If anyone else has this working, I'd love to hear how your application is setup so I can troubleshoot mine some more.


Readme robustness test workflow

The test subject right now is a scaffolding of a Vue app, we're really only interested in the dependencies tree.
Closes https://github.com/stateful/runme/issues/59.

Creating PR so I can test the workflow in Github Actions

Added quite a few more dependencies, not just vuejs, to broaden exposure.


Looking for active maintainer to handle releases

I got in touch with @andrewrk to ask if he could give an active maintainer of this repo the ability to push releases to NPM, as per #139. Unfortunately he could not find any maintainers currently active for this project in that issue.
Is there anyone that has a history with this project, and is willing to handle this role?

thanks @TikiTDO
confirming what you said.

I can

@faceleg Would you be able to do a new release soon? :)


product landing Page Using HTML-CSS & JS

hi @nidhisingh-1  please assign  this reagrding hacktoberfest

Hi @nidhisingh-1 ,
I would like to work on it. Plz assign it to me


Build ERROR: undeclared inclusion(s) in rule '//tensorflow/core/kernels:batchtospace_op_gpu':

NOTE: Only file GitHub issues for bugs and feature requests.  All other topics will be closed.
For general support from the community, see StackOverflow.
To make bugs and feature requests more easy to find and organize, we close issues that are deemed
out of scope for GitHub Issues and point people to StackOverflow.
For bugs or installation issues, please provide the following information.
The more information you provide, the more easily we will be able to offer
help and advice.
What related GitHub issues or StackOverflow threads have you found by searching the web for your problem?
Environment info
Operating System: Linux RHL 7.2
Installed version of CUDA and cuDNN:
(please attach the output of ls -l /path/to/cuda/lib/libcud*): cuda 7.5 + cudnn 5
If installed from binary pip package, provide:

A link to the pip package you installed:
The output from python -c "import tensorflow; print(tensorflow.__version__)".

If installed from source, provide

The commit hash (git rev-parse HEAD)
The output of bazel version

If possible, provide a minimal reproducible example (We usually don't have time to read hundreds of lines of your code)
./configure (select to support GPU)
bazel build -c opt --config=cuda //tensorflow/tools/pip_package:build_pip_package  --verbose_failures
What other attempted solutions have you tried?
Rebuild from scratch, still repro.
Logs or other output that would be helpful
(If logs are large, please upload as attachment or provide link).
ERROR: /data/tools/tensorflow/core/kernels/BUILD:1509:1: undeclared inclusion(s) in rule '//tensorflow/core/kernels:batchtospace_op_gpu':
this rule is missing dependency declarations for the following files included by 'tensorflow/core/kernels/batchtospace_op_gpu.cu.cc':
'/usr/local/cuda-7.5/include/cuda_runtime.h'
'/usr/local/cuda-7.5/include/host_config.h'
'/usr/local/cuda-7.5/include/builtin_types.h'
'/usr/local/cuda-7.5/include/device_types.h'
'/usr/local/cuda-7.5/include/host_defines.h'
'/usr/local/cuda-7.5/include/driver_types.h'
'/usr/local/cuda-7.5/include/surface_types.h'
'/usr/local/cuda-7.5/include/texture_types.h'
'/usr/local/cuda-7.5/include/vector_types.h'
'/usr/local/cuda-7.5/include/channel_descriptor.h'
'/usr/local/cuda-7.5/include/cuda_runtime_api.h'
'/usr/local/cuda-7.5/include/cuda_device_runtime_api.h'
'/usr/local/cuda-7.5/include/driver_functions.h'
'/usr/local/cuda-7.5/include/vector_functions.h'
'/usr/local/cuda-7.5/include/vector_functions.hpp'
'/usr/local/cuda-7.5/include/common_functions.h'
'/usr/local/cuda-7.5/include/math_functions.h'
'/usr/local/cuda-7.5/include/math_functions.hpp'
'/usr/local/cuda-7.5/include/math_functions_dbl_ptx3.h'
'/usr/local/cuda-7.5/include/math_functions_dbl_ptx3.hpp'
'/usr/local/cuda-7.5/include/cuda_surface_types.h'
'/usr/local/cuda-7.5/include/cuda_texture_types.h'
...

@skydoorkai - Are you explicitly specifying the CUDA version when running ./configure? If not, then most likely, you are hitting #3985.

I had this issue and fixed it by explicitly specifying the CUDA version (8.0) when running ./configure.

I'm closing this as a duplicate of #3985.


Form validation does not work without submit button

I have form which should submit data when all fields are filled without any error and submit button is not needed in this case, but validation does not work without submit button. Can validation work without submit button?
"version": "3.11.0"

yes, you can :)
const onSubmit = data => { cosnole.log(data) }

test

Thank you!


New periodic and scheduled effector policies

Adds policies to execute effectors at specific times, either singly or repeated.

cool @grkvlt - wonder if you could do two things:

merge #818 here for testing and follow the pattern there to set trigger and action highlights
include a YAML illustrating use of these policies

(yaml could be used in a test :nudge:)

I have added a PR for the documentation for this here.

@ahgittin I will do some testing of how this interacts with #818 but it will be in a separate PR so that this can get into the 0.12 RC


Add config.jsdomModulePath

Allows for providing a specific version of JSDOM to use in the runner, rather than the version Jest currently has pulled in as a dep.
Use case:

JSDOM updates to a new major version (9.x)
Install it in your project as normal, npm i jsdom@latest --save-dev
Add this to your package.json "jest" entry: "jsdomModulePath": "node_modules/jsdom"
Remove it later on (or not) when Jest updates to it by default

I think we are gonna go with #720 instead as @yaycmyk and I discussed a few days ago. This is mostly to not incur technical debt. I'll clean up all related tests at FB that might fail as a result of this.

FWIW, I still think this is would be a useful addition to Jest. It's a minor configuration option that would add little technical debt and makes an integral aspect of the testing environment more flexible.

The idea is to make jest more modular and at some point make the jsdom environment its own npm package. You can then opt to install your own version of it if you wish and also we can add more options to that. Right now, you can specify testEnvironment with a full path and that can be your own implementation of JSDOMEnvironment. I think that should be enough.


cargo miri: can't disable colors

It seems that since the last miri sync, stdout/stderr is printed with colors unconditionally?
This broke a bunch of icemaker regexes which fetch UB/ICEs from miri stdout and produced a couple of funny diffs:
-    "error_reason": "error: Undefined Behavior: type validation failed at .value: encountered uninitialized raw pointer",
+    "error_reason": "\u001b[0m   \u001b[0m\u001b[0m\u001b[1m\u001b[38;5;12m= \u001b[0m\u001b[0m\u001b[1mhelp\u001b[0m\u001b[0m: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior\u001b[0m",

I tried to disable colors but not even cargo --color=never miri run  --color=never does work.

How does cargo forward --color=never to rustc? I would hope it stops passing diagnostic-rendered-ansi...

Oh no. Cargo seems to always pass --json=diagnostic-rendered-ansi,artifacts,future-incompat, even with --color=never. Does cargo then itself strip the ANSI from the JSON before putting it to the terminal...?
It looks like there is no way for the rustc driver to tell whether the user wanted output with or without colors? Cc @ehuss

For caching purposes, cargo strips out the ANSI when color is disabled.


AUR package published

Not sure where else to announce this, i built a simple PKGBUILD to create and install an archlinux package for this lib. Maybe an Install/Building section would be good?
https://aur.archlinux.org/packages/rpgp-git/

Cool, does this require republishing on every change?

Currently yes because i do a shallow clone and can't get the latest tag (git tags are not included in a shallow clone). Therefore i hardcoded it.

On Wed, May 22, 2019 at 11:42 -0700, Jikstra wrote:

Currently yes because i do a shallow clone and can't get the latest tag (git tags are not included in a shallow clone). Therefore i hardcoded it.

for me cloning rpgp takes 3-4 secs but apparently for jikstra it's different
which is why he does only a shallow clone.

that's fixed by now IIRC.  if not @jikstra, please speak up and re-open/open fresh.


set/unset some security related headers in nginx

clear X-Forwarded-Host: if it was set to www.evil.com, and we 302 to /foo,
the browser would redirect to www.evil.com/foo
add X-Content-Type-Options nosniff: stop browsers inferring MIME types, which
is bad and stuff.
set X-Xss-Protection, which is enabled by default in most browsers and
completely stops rendering if XSS is detected
for more information on the headers, have a look at all the useful links in
https://gist.github.com/plentz/6737338
we've already got Strict-Transport-Security enabled everywhere other than the healthcheck conf (https://github.com/alphagov/digitalmarketplace-aws/blob/master/playbooks/roles/nginx/templates/healthcheck.j2)

Should we move the HSTS one to nginx.conf too? I think it's repeated in each server config at the moment and it'd be nice to keep the security stuff all together

provided it won't cause an issue to the healthchecks, which might be http? but i can't imagine it being an issue cos it's only a suggestion to the browser that it should use https instead, rather than a strict command. i'll do it and we can see if everything explodes i guess¯\_(ツ)_/¯

👍


Use message from the FindBoost.cmake module

Thanks - can you please squash the two commits so that we get a nicer history?

Done.


added documentation for TestCaseFilter

@ffMathy : Thanks for your contribution! The author(s) have been notified to review your proposed change.

@pavelhorak @AbhitejJohn to check fix

I believe is correct. @nohwnd can you confirm? Would it make sense adding TestCaseFilter attribute into RunConfiguration element example?

See this. That's when it was merged.
https://github.com/microsoft/vstest/pull/2356

It looks like it was undocumented on purpose according to #2356. However, I think it's a great feature and workaround for the issue here: https://github.com/microsoft/vstest/issues/2625

Yup, undocumented on purpose.

@nohwnd Closing based on preceding comments. Let me know if I missed something. Thanks!


feat: with no mail accounts, the wizard should be opened automatically

method areAccountsLoaded() now returns the state of the store, i.e. true
if a successful attempt was made to load the accounts.


TEP-0135: Introduce coscheduling feature flags

Part of #6740. TEP-0135 introduces a feature that allows a cluster operator to ensure that all of a PipelineRun's pods are scheduled to the same node.
This commit introduces a new feature flag coscheduling which works together with the disable-affinity-assistant feature flag to determine the Affinity Assistant behavior. The usage of the new feature flag will be added in the follow-up PRs.
The details of the coscheduling feature flag can be found in the Configuration section of TEP-0135. The details of the disable-affinity-assistant feature flag can be found in the Upgrade and Migration Strategy section of TEP-0135.
NOTE: this feature is WIP, please do not use on this feature.
/kind feature

Changes

Submitter Checklist
As the author of this PR, please check off the items in this checklist:

[ ] Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
[ ] Has Tests included if any functionality added or changed
[ ] Follows the commit message standard
[ ] Meets the Tekton contributor standards (including functionality, content, code)
[ ] Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
[ ] Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings). See some examples of good release notes.
[ ] Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes
tep-0135: introduce `coscheduling` feature flag

/test pull-tekton-pipeline-build-tests

/test pull-tekton-pipeline-unit-tests

/test tekton-pipeline-unit-tests

@pritidesai did you want to weigh in on naming of this feature flag?
I think "coscheduling" is more accurate than "coexisting", since this feature affects scheduling behavior, but we can definitely play around with options that are more concise!

I do not think so. This feature does affect scheduling behavior but in the end the pipelineRuns coexist. But I am open to go with coscheduling v/s coexisting.  Please consider naming the values a bit more concise and accurate like it was discussed in the community PR. Thanks!

@pritidesai did you want to weigh in on naming of this feature flag?
I think "coscheduling" is more accurate than "coexisting", since this feature affects scheduling behavior, but we can definitely play around with options that are more concise!

I do not think so. This feature does affect scheduling behavior but in the end the pipelineRuns coexist. But I am open to go with coscheduling v/s coexisting. Please consider naming the values a bit more concise and accurate like it was discussed in the community PR. Thanks!

I personally don't feel coscheduling is confusing, but I don't have a strong preference around coscheduling v/s coexisting as long as we have clear documentation. I am open to make the name more concise by removing the duplicated coschedule as @afrittoli suggested in https://github.com/tektoncd/community/pull/1025#discussion_r1231296806 (changing pipelines -> pipelineruns).
That is:
flag name: coschedule
values: [disabled, workspaces, pipelineruns, isolate-pipelineruns]
WDYT? @lbernick?

"coscheduling" with options [disabled, workspaces, pipelineruns, isolate-pipelineruns] sgtm

"coscheduling" with options [disabled, workspaces, pipelineruns, isolate-pipelineruns] sgtm

Cool, sounds like we are close to a commonly accepted solution! I will start with updating the TEP!

/hold until https://github.com/tektoncd/community/pull/1031 is merged

uncovered error

Hi @jerop! This line is actually not reachable because I cannot pass in an invalid feature flag combination to this function and we have validation in cfgtesting.SetFeatureFlags().
Please see the discussion here: https://github.com/tektoncd/pipeline/pull/6790#discussion_r1222107970

/lgtm


WIP: try making cargo tree's Graph from a UnitGraph

What does this PR try to resolve?
This is an attempt at implementing step 1 from https://github.com/rust-lang/cargo/issues/9599#issuecomment-1312587071 - a function which converts from a UnitGraph to a cargo::ops::tree::graph::Graph
This would make it easier to find edge cases where cargo tree disagrees with cargo build.
How should we test and review this PR?
TODO (this PR is currently WIP)
Demonstrate how you test this change and guide reviewers through your PR.
With a smooth review process, a pull request usually gets reviewed quicker.
If you don't know how to write and run your tests, please read the guide:
https://doc.crates.io/contrib/tests
Additional information

[x] minimal test

[ ] think a bit harder about how/whether to expose this code (currently hidden behind a CARGO_TREE_FROM_UNIT_GRAPH environment variable for simplicity)

[x] actually implement Graph::from_bcx() (to create a Graph from a UnitGraph)
[ ] run everything in the current test suite using Graph::from_bcx() rather than graph::build() and fix any discrepancies
[ ] get feedback on whether the team likes this approach (if not then that's fine - I need to build something like this for cargo quickbuild anyway, so it won't be wasted effort).

Progress report for today:
my simple_from_unit_graph test now passes (so top-level build-deps and dev-deps are now supported)
If I apply this patch:
diff --git a/crates/cargo-test-support/src/lib.rs b/crates/cargo-test-support/src/lib.rs
index 4ce0162bf..513d4f4f8 100644
--- a/crates/cargo-test-support/src/lib.rs
+++ b/crates/cargo-test-support/src/lib.rs
@@ -414,6 +414,7 @@ impl Project {
         let mut execs = self.process(&cargo);
         if let Some(ref mut p) = execs.process_builder {
             p.env("CARGO", cargo);
+            p.env("CARGO_TREE_FROM_UNIT_GRAPH", "1");
             p.arg_line(cmd);
         }
         execs

then it causes failures from the following 25 tests:
    features2::minimal_download
    features2::tree_all
    features::same_name
    features_namespaced::tree
    features_namespaced::tree_no_implicit
    tree::cyclic_features
    tree::dep_kinds
    tree::depth_limit
    tree::dev_dep_cycle_with_feature
    tree::dev_dep_cycle_with_feature_nested
    tree::dev_dep_feature
    tree::duplicates
    tree::features
    tree::format
    tree::host_dep_feature
    tree::invert_with_build_dep
    tree::prune
    tree::virtual_workspace
    tree::workspace_features_are_local
    tree_graph_features::dep_feature_various
    tree_graph_features::features_enables_inactive_target
    tree_graph_features::graph_features_ws_interdependent
    tree_graph_features::slash_feature_name
    weak_dep_features::tree
    weak_dep_features::weak_namespaced

Something to work on tomorrow.

Progress report for the weekend:
Apart from the tests that pass --target=all, I only have about 3 failing tests. I can probably chip away at these, but I'm not sure what to do about --target=all.
Questions:
a) Do you think that it's worth trying to teach cargo::core::compiler::unit_dependencies about --target=all or is it possible to deprecate it?
b) cargo tree -p $package seems to mean something slightly different from cargo build -p $package (cargo tree seems to use the whole workspace for calculating the dep tree, and then print a filtered tree by using $package as the root). Do I have this right, or do I just have bugs in my code/understanding? I'm tempted to make a cargo build --stop-after-package $package that does the same thing as cargo tree -p $package. Does this seem like a reasonable idea?
c) at the moment, I'm only adding complexity (because the existing code is useful as an oracle), but in theory this approach would let us remove a bunch of complexity by removing the existing Graph-building machinery. Is this a thing we want to do? Should I have a stab at it, and see what my diffstat looks like with the old code removed?
d) is this PR fleshed-out enough to determine whether we even want to go down this route? Is there anything else that would make it easier to make that decision?
I doubt that this PR will be in a reasonable state to merge before I get swallowed up by my new job, but it will hopefully serve as a reference of what is possible and what is easier/harder if you go down this route. I will therefore close this PR on Tuesday, whatever state it's in. I will leave the branch around in case someone wants to use it as inspiration.

Haven't really looked deeply into your change, but I do appreciate what you've done. The amount of code added so far are less than I expected. It is indeed a great exploration. Thank you!

a) Do you think that it's worth trying to teach cargo::core::compiler::unit_dependencies about --target=all or is it possible to deprecate it?

I don't think so, at least for the latter. They have different meanings. We already have ForceAllTargets to disable target platform filters. Maybe we can look into that without too many churns on the other parts of code?

b) cargo tree -p $package seems to mean something slightly different from cargo build -p $package (cargo tree seems to use the whole workspace for calculating the dep tree, and then print a filtered tree by using $package as the root). Do I have this right, or do I just have bugs in my code/understanding? I'm tempted to make a cargo build --stop-after-package $package that does the same thing as cargo tree -p $package. Does this seem like a reasonable idea?

Package filtering process happens here, so ws_resolve should resolve what is passed incargo tree -p. Did you find any abnormal behaviour? For --stop-after-package, I am not sure what it is used for, though usually Cargo team takes it serious when adding a new flag.

c) at the moment, I'm only adding complexity (because the existing code is useful as an oracle), but in theory this approach would let us remove a bunch of complexity by removing the existing Graph-building machinery. Is this a thing we want to do? Should I have a stab at it, and see what my diffstat looks like with the old code removed?

Go ahead, or put it into another module, so we can have a clear view of the diff :)

d) is this PR fleshed-out enough to determine whether we even want to go down this route? Is there anything else that would make it easier to make that decision?

Even the final result reduces the complexity, I'll still ask about the extensibility, such as handling an issue like https://github.com/rust-lang/cargo/issues/10593#issuecomment-1317759526. I think ehuss has some concerns on sharing code, so having ehuss taking a look is better once it is in a relatively good shape.

Haven't really looked deeply into your change, but I do appreciate what you've done. The amount of code added so far are less than I expected. It is indeed a great exploration. Thank you!

a) Do you think that it's worth trying to teach cargo::core::compiler::unit_dependencies about --target=all or is it possible to deprecate it?

I don't think so, at least for the latter. They have different meanings. We already have ForceAllTargets to disable target platform filters. Maybe we can look into that without too many churns on the other parts of code?

Ah. I think I might have misunderstood that part of the code. For some reason, I thought ForceAllTargets was about the --all-targets flag (equivalent to specifying --lib --bins --tests --benches --examples). Thanks. I'll have a go at using that machinery.

b) cargo tree -p $package seems to mean something slightly different from cargo build -p $package (cargo tree seems to use the whole workspace for calculating the dep tree, and then print a filtered tree by using $package as the root). Do I have this right, or do I just have bugs in my code/understanding? I'm tempted to make a cargo build --stop-after-package $package that does the same thing as cargo tree -p $package. Does this seem like a reasonable idea?

Package filtering process happens here, so ws_resolve should resolve what is passed incargo tree -p. Did you find any abnormal behaviour? For --stop-after-package, I am not sure what it is used for, though usually Cargo team takes it serious when adding a new flag.

I think this may be due to bugs in my understanding again. I was under the impression that it was possible to ask cargo tree to "show me how cargo would build this workspace, but only show me things below $package" in the tree.

[ ] dig into the the tests that I chalked up to this discrepancy and see if I can resolve them.

c) at the moment, I'm only adding complexity (because the existing code is useful as an oracle), but in theory this approach would let us remove a bunch of complexity by removing the existing Graph-building machinery. Is this a thing we want to do? Should I have a stab at it, and see what my diffstat looks like with the old code removed?

Go ahead, or put it into another module, so we can have a clear view of the diff :)

I will do the "another module" thing first, and then do "deleting the old code" as a separate PR against my own fork if I get time.

d) is this PR fleshed-out enough to determine whether we even want to go down this route? Is there anything else that would make it easier to make that decision?

Even the final result reduces the complexity, I'll still ask about the extensibility, such as handling an issue like #10593 (comment). I think ehuss has some concerns on sharing code, so having ehuss taking a look is better once it is in a relatively good shape.

I suppose this is a philosophical thing: is cargo tree a teaching/exploring tool (in which case, clarity and expressiveness might be more important) or is it a debugging tool (in which case correctness might be more important).
Fundamentally, cargo tree does not work on the Unit level, and there will always be discrepancies between it and cargo build. This is something that I didn't appreciate when I started this project. It will certainly help when I next have time to work on cargo quickbuild.

I've reached the end of my timebox for this.
I briefly tried propagating ForceAllTargets, but I ran out of time before I got any --target=all tests passing. I also didn't get time to do any cleaning up, so it's still a mess. Sorry.
I still think that this is a promising idea.
Advantages:

The Graph building code in graph::from_bcx() does not need to be recursive anymore. This means that it is easier to follow, and doesn't have to worry about infinite loops.

It does a pass to add all of the Package nodes, then a pass to add all of the links (which may add Feature nodes if feature edges are enabled)
The pass to add cli features at the end is currently still the old recursive add_internal_features() code, but it might be possible to flatten that out too (not sure).

We now match the behaviour of UnitGraph - build-dependencies are ignored if there is no build.rs unit.

As an aside, I wonder if we could use a similar approach to support displaying artifact dependencies and build-scripts. You would want a -e units or something, and a Node::Unit variant, a bit like how Node::Feature works (I have not thought about how it would interact with -e features). Going via UnitGraph for that feature would make things a lot easier, so if you are already going via UnitGraph for everything else, you might have an advantage.
Risks:

I didn't have time to look into the --target=all tests, and a few other things, so there are 6 failing tests. Any one of them might be difficult enough to solve that it kills this idea dead.
we still rely on calling out to the resolver to work out why each of the UnitDeps are there, because they don't contain enough information on their own. This could be considered to be a bad thing, but also may be considered to be a good thing, if we need extension points.
I'm still using a few of the old builder functions, like add_internal_features(), and they might be correcting for sloppiness in how I'm building the tree. It might be valuable to rewrite them in terms of UnitGraph to make sure we're not missing anything.
Please don't let my prototype-quality code convince you that it's a bad idea: it is my first attempt at hacking on cargo internals, so it's a bit of a mess.

Thanks for all of your support in this exploration. Hopefully it contains some useful ideas. Even if it doesn't, I have learned a lot.
Thanks again.
David.


Add (more generic) support for Circl signature schemes

Example usage

I'll check this tomorrow as well ;)

Updated with API changes of @armfazh

Looking good for me @bwesterb ! What is it missing?

Looking good for me @bwesterb ! What is it missing?

I'm waiting for https://github.com/cloudflare/circl/pull/143 to be merged.