diff options
20 files changed, 787 insertions, 127 deletions
diff --git a/cmake/QtBuildRepoHelpers.cmake b/cmake/QtBuildRepoHelpers.cmake index 1429448c2f8..5961424c38b 100644 --- a/cmake/QtBuildRepoHelpers.cmake +++ b/cmake/QtBuildRepoHelpers.cmake @@ -724,6 +724,41 @@ macro(qt_internal_find_standalone_test_config_file) endif() endmacro() +# Used inside the standalone parts config file to find all requested Qt module packages. +# standalone_parts_args_var_name should be the var name in the outer scope that contains +# all the arguments for this function. +macro(qt_internal_find_standalone_parts_qt_packages standalone_parts_args_var_name) + set(__standalone_parts_opt_args "") + set(__standalone_parts_single_args "") + set(__standalone_parts_multi_args + QT_MODULE_PACKAGES + ) + cmake_parse_arguments(__standalone_parts + "${__standalone_parts_opt_args}" + "${__standalone_parts_single_args}" + "${__standalone_parts_multi_args}" + ${${standalone_parts_args_var_name}}) + + # Packages looked up in standalone tests Config files should use the same version as + # the one recorded on the Platform target. + qt_internal_get_package_version_of_target(Platform __standalone_parts_main_qt_package_version) + + if(__standalone_parts_QT_MODULE_PACKAGES) + foreach(__standalone_parts_package_name IN LISTS __standalone_parts_QT_MODULE_PACKAGES) + find_package(${QT_CMAKE_EXPORT_NAMESPACE} + "${__standalone_parts_main_qt_package_version}" + COMPONENTS "${__standalone_parts_package_name}") + endforeach() + endif() + + unset(__standalone_parts_opt_args) + unset(__standalone_parts_single_args) + unset(__standalone_parts_multi_args) + unset(__standalone_parts_QT_MODULE_PACKAGES) + unset(__standalone_parts_main_qt_package_version) + unset(__standalone_parts_package_name) +endmacro() + # Used by standalone tests and standalone non-ExternalProject examples to find all installed qt # packages. macro(qt_internal_find_standalone_parts_config_files) diff --git a/cmake/QtFeature.cmake b/cmake/QtFeature.cmake index c4564cfb38d..9bbf6e700b4 100644 --- a/cmake/QtFeature.cmake +++ b/cmake/QtFeature.cmake @@ -1302,7 +1302,10 @@ function(qt_feature_module_end) # Before, we didn't use to export the properties at all for INTERFACE_ libraries, # but we need to, because certain GlobalPrivate modules have features which are used # in configure-time conditions for tests. - qt_internal_add_genex_properties_export("${target}" ${properties_to_export}) + qt_internal_add_custom_properties_to_export("${target}" + PROPERTIES_WITHOUT_GENEXES + ${properties_to_export} + ) else() set(propertyPrefix "") set_property(TARGET "${target}" diff --git a/cmake/QtPostProcessHelpers.cmake b/cmake/QtPostProcessHelpers.cmake index 12f5c617960..b8e46085a98 100644 --- a/cmake/QtPostProcessHelpers.cmake +++ b/cmake/QtPostProcessHelpers.cmake @@ -819,7 +819,7 @@ function(qt_internal_create_config_file_for_standalone_tests) # standalone tests, and it can happen that Core or Gui features are not # imported early enough, which means FindWrapPNG will try to find a system PNG library instead # of the bundled one. - set(modules) + set(modules "") foreach(m ${QT_REPO_KNOWN_MODULES}) get_target_property(target_type "${m}" TYPE) @@ -835,12 +835,9 @@ function(qt_internal_create_config_file_for_standalone_tests) endif() endforeach() - list(JOIN modules " " QT_REPO_KNOWN_MODULES_STRING) - string(STRIP "${QT_REPO_KNOWN_MODULES_STRING}" QT_REPO_KNOWN_MODULES_STRING) - # Skip generating and installing file if no modules were built. This make sure not to install # anything when build qtx11extras on macOS for example. - if(NOT QT_REPO_KNOWN_MODULES_STRING) + if(NOT modules) return() endif() @@ -848,8 +845,8 @@ function(qt_internal_create_config_file_for_standalone_tests) # of the current repo. This is used for standalone tests. qt_internal_get_standalone_parts_config_file_name(tests_config_file_name) - # Standalone tests Config files should follow the main versioning scheme. - qt_internal_get_package_version_of_target(Platform main_qt_package_version) + # Substitution variables. + list(JOIN modules "\n " QT_MODULE_PACKAGES) configure_file( "${QT_CMAKE_DIR}/QtStandaloneTestsConfig.cmake.in" diff --git a/cmake/QtStandaloneTestsConfig.cmake.in b/cmake/QtStandaloneTestsConfig.cmake.in index 39200167a58..9d548d14699 100644 --- a/cmake/QtStandaloneTestsConfig.cmake.in +++ b/cmake/QtStandaloneTestsConfig.cmake.in @@ -1,8 +1,8 @@ # Copyright (C) 2024 The Qt Company Ltd. # SPDX-License-Identifier: BSD-3-Clause -# TODO: Ideally this should look for each Qt module separately, with each module's specific version, -# bypassing the Qt6 Config file, aka find_package(Qt6SpecificFoo) repated x times. But it's not -# critical. -find_package(@INSTALL_CMAKE_NAMESPACE@ @main_qt_package_version@ - COMPONENTS @QT_REPO_KNOWN_MODULES_STRING@) +set(__standalone_parts_qt_packages_args + QT_MODULE_PACKAGES + @QT_MODULE_PACKAGES@ +) +qt_internal_find_standalone_parts_qt_packages(__standalone_parts_qt_packages_args) diff --git a/cmake/QtTargetHelpers.cmake b/cmake/QtTargetHelpers.cmake index eedfdbeba74..397628ba11a 100644 --- a/cmake/QtTargetHelpers.cmake +++ b/cmake/QtTargetHelpers.cmake @@ -1688,15 +1688,35 @@ function(qt_internal_get_target_sources_property out_var) set(${out_var} "${${out_var}}" PARENT_SCOPE) endfunction() -# This function collects target properties that contain generator expressions and needs to be -# exported. This function is needed since the CMake EXPORT_PROPERTIES property doesn't support -# properties that contain generator expressions. -# Usage: qt_internal_add_genex_properties_export(target properties...) -function(qt_internal_add_genex_properties_export target) +# This function collects target properties that need to be exported without using CMake's +# EXPORT_PROPERTIES. +# Use cases: +# - Properties named INTERFACE_foo (which CMake doesn't allow exporting) +# - Properties that contain generator expressions (need special handling for multi-config builds) +# Usage: +# qt_internal_add_custom_properties_to_export(target +# PROPERTIES property1 [property2 ...] +# PROPERTIES_WITHOUT_GENEXES property3 [property4 ...] +# ) +# Arguments: +# PROPERTIES +# should contain names of properties that can differ in multi-config builds (e.g. paths) +# PROPERTIES_WITHOUT_GENEXES +# should contain names of properties that will always have the same value in multi config +# builds (e.g, feature values). +function(qt_internal_add_custom_properties_to_export target) + set(opt_args "") + set(single_args "") + set(multi_args + PROPERTIES + PROPERTIES_WITHOUT_GENEXES + ) + cmake_parse_arguments(PARSE_ARGV 1 arg "${opt_args}" "${single_args}" "${multi_args}") + _qt_internal_validate_all_args_are_parsed(arg) + get_cmake_property(is_multi_config GENERATOR_IS_MULTI_CONFIG) - set(config_check_begin "") - set(config_check_end "") + # Prepare multi-config helper genexes. if(is_multi_config) list(GET CMAKE_CONFIGURATION_TYPES 0 first_config_type) @@ -1704,7 +1724,7 @@ function(qt_internal_add_genex_properties_export target) # The check is only applicable to the 'main' configuration. If user project doesn't use # multi-config generator, then the check supposed to return true and the value from the # 'main' configuration supposed to be used. - string(JOIN "" check_if_config_empty + string(CONCAT check_if_config_empty "$<1:$><NOT:" "$<1:$><BOOL:" "$<1:$><CONFIG$<ANGLE-R>" @@ -1714,7 +1734,7 @@ function(qt_internal_add_genex_properties_export target) # The genex snippet is evaluated to '$<CONFIG:'Qt config type'>' in the generated cmake # file and checks if the config that user uses matches the generated cmake file config. - string(JOIN "" check_user_config + string(CONCAT check_user_config "$<1:$><CONFIG:$<CONFIG>$<ANGLE-R>" ) @@ -1725,34 +1745,70 @@ function(qt_internal_add_genex_properties_export target) # user project according to the user config type. # All genexes need to be escaped properly to protect them from evaluation by the # file(GENERATE call in the qt_internal_export_genex_properties function. - string(JOIN "" config_check_begin + string(CONCAT config_check_begin_multi "$<1:$><" "$<1:$><OR:" "${check_user_config}" "$<$<CONFIG:${first_config_type}>:$<COMMA>${check_if_config_empty}>" "$<ANGLE-R>:" ) - set(config_check_end "$<ANGLE-R>") + set(config_check_end_multi "$<ANGLE-R>") endif() set(target_name "${QT_CMAKE_EXPORT_NAMESPACE}::${target}") - foreach(property IN LISTS ARGN) - set(target_property_genex "$<TARGET_PROPERTY:${target_name},${property}>") - # All properties that contain lists need to be protected of processing by JOIN genex calls. - # So this escapes the semicolons for these list. - set(target_property_list_escape - "$<JOIN:$<GENEX_EVAL:${target_property_genex}>,\;>") - set(property_value - "\"${config_check_begin}${target_property_list_escape}${config_check_end}\"") - set_property(TARGET ${target} APPEND PROPERTY _qt_export_genex_properties_content - "${property} ${property_value}") + + set(property_sources + PROPERTIES + PROPERTIES_WITHOUT_GENEXES + ) + + foreach(property_source IN LISTS property_sources) + if(property_source STREQUAL "PROPERTIES") + # Properties with genexes need multi-config specific handling. + set(config_check_begin "${config_check_begin_multi}") + set(config_check_end "${config_check_end_multi}") + + set(output_property "_qt_export_custom_properties_content") + elseif(property_source STREQUAL "PROPERTIES_WITHOUT_GENEXES") + # Properties without genexes don't need the config checks. + set(config_check_begin "") + set(config_check_end "") + + set(output_property "_qt_export_custom_properties_no_genexes_content") + else() + message(FATAL_ERROR "Invalid type of property source" ${property_source}"") + endif() + + foreach(property IN LISTS arg_${property_source}) + set(target_property_genex "$<TARGET_PROPERTY:${target_name},${property}>") + # All properties that contain lists need to be protected of processing by JOIN genex + # calls. So this escapes the semicolons for these list. + set(target_property_list_escape + "$<JOIN:$<GENEX_EVAL:${target_property_genex}>,\;>") + set(property_value + "\"${config_check_begin}${target_property_list_escape}${config_check_end}\"") + set_property(TARGET ${target} APPEND PROPERTY "${output_property}" + "${property} ${property_value}") + endforeach() endforeach() endfunction() -# This function executes generator expressions for the properties that are added by the -# qt_internal_add_genex_properties_export function and sets the calculated values to the -# corresponding properties in the generated ExtraProperties.cmake file. The file then needs to be -# included after the target creation routines in Config.cmake files. It also supports Multi-Config -# builds. +# This function generates and installs ${EXPORT_NAME_PREFIX}ExportProperties-$<CONFIG>.cmake files +# to be included from inside a FooConfig.cmake file. +# +# The file contains set_property(TARGET PROPERTY) assignments that append values to a given target's +# properties as added by the qt_internal_add_custom_properties_to_export function. +# +# The assigned values are computed from the result of executing the generator expressions that were +# stored in the properties, and are wrapped in config-specific genexes in a multi-config build. +# +# Example output: +# set_property(TARGET Qt6::Foo PROPERTY MY_GENEX_PROP +# "$<$<OR:$<CONFIG:RelWithDebInfo>,$<NOT:$<BOOL:$<CONFIG>>>>:OneReleaseVal>") +# set_property(TARGET Qt6::Foo PROPERTY MY_REGULAR_PROP "SecondValue") +# include("${CMAKE_CURRENT_LIST_DIR}/Qt6FooExtraProperties-Debug.cmake") +# inside the include +# set_property(TARGET Qt6::Foo APPEND PROPERTY MY_GENEX_PROP "$<$<OR:$<CONFIG:Debug>>:OneDebugVal>") +# # Arguments: # EXPORT_NAME_PREFIX: # The portion of the file name before ExtraProperties.cmake @@ -1761,13 +1817,15 @@ endfunction() # TARGETS: # The internal target names. function(qt_internal_export_genex_properties) - set(option_args "") + set(opt_args "") set(single_args EXPORT_NAME_PREFIX CONFIG_INSTALL_DIR ) - set(multi_args TARGETS) - cmake_parse_arguments(arg "${option_args}" "${single_args}" "${multi_args}" ${ARGN}) + set(multi_args + TARGETS + ) + cmake_parse_arguments(PARSE_ARGV 0 arg "${opt_args}" "${single_args}" "${multi_args}") if(NOT arg_EXPORT_NAME_PREFIX) message(FATAL_ERROR "qt_internal_export_genex_properties: " @@ -1779,24 +1837,36 @@ function(qt_internal_export_genex_properties) "TARGETS argument must contain at least one target") endif() - foreach(target IN LISTS arg_TARGETS) - get_cmake_property(is_multi_config GENERATOR_IS_MULTI_CONFIG) + # TODO: Handling more than one target won't work correctly atm due to trying to create and + # install the same file name multiple times for each target. + list(LENGTH arg_TARGETS targets_count) + if(targets_count GREATER 1) + message(AUTHOR_WARNING "qt_internal_export_genex_properties: " + "Specifying more than one target is not fully supported yet.") + endif() + get_cmake_property(is_multi_config GENERATOR_IS_MULTI_CONFIG) + + set(should_append "") + set(config_suffix "") + set(is_first_config "1") + if(is_multi_config) + list(GET CMAKE_CONFIGURATION_TYPES 0 first_config_type) + + # The non-genex properties should only go to the first config file. + set(is_first_config "$<CONFIG:${first_config_type}>") + + set(config_suffix "$<$<NOT:${is_first_config}>:-$<CONFIG>>") + # If the generated file belongs to the 'main' config type, we should set property + # but not append it. + string(JOIN "" should_append + "$<$<NOT:${is_first_config}>: APPEND>") + endif() + + foreach(target IN LISTS arg_TARGETS) set(output_file_base_name "${arg_EXPORT_NAME_PREFIX}ExtraProperties") - set(should_append "") - set(config_suffix "") - if(is_multi_config) - list(GET CMAKE_CONFIGURATION_TYPES 0 first_config_type) - set(config_suffix "$<$<NOT:$<CONFIG:${first_config_type}>>:-$<CONFIG>>") - # If the generated file belongs to the 'main' config type, we should set property - # but not append it. - string(JOIN "" should_append - "$<$<NOT:$<CONFIG:${first_config_type}>>: APPEND>") - endif() set(file_name "${output_file_base_name}${config_suffix}.cmake") - - qt_path_join(output_file "${arg_CONFIG_INSTALL_DIR}" - "${file_name}") + qt_path_join(output_file "${arg_CONFIG_INSTALL_DIR}" "${file_name}") if(NOT IS_ABSOLUTE "${output_file}") qt_path_join(output_file "${QT_BUILD_DIR}" "${output_file}") @@ -1804,17 +1874,51 @@ function(qt_internal_export_genex_properties) set(target_name "${QT_CMAKE_EXPORT_NAMESPACE}::${target}") + # Common genex helpers. string(JOIN "" set_property_begin "set_property(TARGET " "${target_name}${should_append} PROPERTY " ) set(set_property_end ")") set(set_property_glue "${set_property_end}\n${set_property_begin}") + set(t_prop "TARGET_PROPERTY:${target}") + + # Handle the properties that contain genexes. set(property_list - "$<GENEX_EVAL:$<TARGET_PROPERTY:${target},_qt_export_genex_properties_content>>") - string(JOIN "" set_property_content "${set_property_begin}" + "$<GENEX_EVAL:$<${t_prop},_qt_export_custom_properties_content>>") + set(property_has_values "$<BOOL:${property_list}>") + string(CONCAT set_property_content + "${set_property_begin}" "$<JOIN:${property_list},${set_property_glue}>" "${set_property_end}") + string(CONCAT set_property_content_conditional + "$<${property_has_values}:" + "\n${set_property_content}" + ">") + + # We need to ensure the no genexes content only gets added to the first config file. + set(property_no_genexes_list + "$<GENEX_EVAL:$<${t_prop},_qt_export_custom_properties_no_genexes_content>>") + set(property_no_genexes_has_values "$<BOOL:${property_no_genexes_list}>") + string(CONCAT property_no_genexes_has_values_and_first_config + "$<AND:${property_no_genexes_has_values},${is_first_config}>") + + string(CONCAT set_property_no_genexes_content + "${set_property_begin}" + "$<JOIN:${property_no_genexes_list},${set_property_glue}>" + "${set_property_end}") + + string(CONCAT set_property_no_genexes_content_conditional + "$<${property_no_genexes_has_values_and_first_config}:" + "\n${set_property_no_genexes_content}" + ">") + + # Final content is generated if at least one genex-carrying property has a value, + # or if we are in the first config and at least one no-genex property has a value. + set(content_available_condition + "$<OR:${property_has_values},${property_no_genexes_has_values_and_first_config}>") + + set(config_includes_string "") if(is_multi_config) set(config_includes "") foreach(config IN LISTS CMAKE_CONFIGURATION_TYPES) @@ -1827,19 +1931,33 @@ function(qt_internal_export_genex_properties) endforeach() list(JOIN config_includes "\n" config_includes_string) set(config_includes_string - "\n$<$<CONFIG:${first_config_type}>:${config_includes_string}>") + "\n$<${is_first_config}:${config_includes_string}>") + + # Config includes should be included if we have properties with genexes, which are + # config specific. + string(CONCAT config_includes_string_conditional + "$<${property_has_values}:" + "${config_includes_string}" + ">") endif() + string(CONCAT final_content + "$<${content_available_condition}:" + "${set_property_content_conditional}" + "${set_property_no_genexes_content_conditional}" + "${config_includes_string_conditional}" + ">") + file(GENERATE OUTPUT "${output_file}" - CONTENT "$<$<BOOL:${property_list}>:${set_property_content}${config_includes_string}>" - CONDITION "$<BOOL:${property_list}>" + CONTENT "${final_content}" + CONDITION "${content_available_condition}" ) - endforeach() - qt_install(FILES "$<$<BOOL:${property_list}>:${output_file}>" - DESTINATION "${arg_CONFIG_INSTALL_DIR}" - COMPONENT Devel - ) + qt_install(FILES "$<${content_available_condition}:${output_file}>" + DESTINATION "${arg_CONFIG_INSTALL_DIR}" + COMPONENT Devel + ) + endforeach() endfunction() # A small wrapper for adding the Platform target, and a building block for the PlatformXInternal diff --git a/src/corelib/kernel/qmetacontainer.cpp b/src/corelib/kernel/qmetacontainer.cpp index 4b4ea06d8b9..6173198a972 100644 --- a/src/corelib/kernel/qmetacontainer.cpp +++ b/src/corelib/kernel/qmetacontainer.cpp @@ -210,7 +210,7 @@ void QMetaContainer::destroyIterator(const void *iterator) const */ bool QMetaContainer::compareIterator(const void *i, const void *j) const { - return hasIterator() ? d_ptr->compareIteratorFn(i, j) : false; + return i == j || (hasIterator() && d_ptr->compareIteratorFn(i, j)); } /*! @@ -249,7 +249,7 @@ void QMetaContainer::advanceIterator(void *iterator, qsizetype step) const */ qsizetype QMetaContainer::diffIterator(const void *i, const void *j) const { - return hasIterator() ? d_ptr->diffIteratorFn(i, j) : 0; + return (i != j && hasIterator()) ? d_ptr->diffIteratorFn(i, j) : 0; } /*! @@ -327,7 +327,7 @@ void QMetaContainer::destroyConstIterator(const void *iterator) const */ bool QMetaContainer::compareConstIterator(const void *i, const void *j) const { - return hasConstIterator() ? d_ptr->compareConstIteratorFn(i, j) : false; + return i == j || (hasConstIterator() && d_ptr->compareConstIteratorFn(i, j)); } /*! @@ -366,7 +366,7 @@ void QMetaContainer::advanceConstIterator(void *iterator, qsizetype step) const */ qsizetype QMetaContainer::diffConstIterator(const void *i, const void *j) const { - return hasConstIterator() ? d_ptr->diffConstIteratorFn(i, j) : 0; + return (i != j && hasConstIterator()) ? d_ptr->diffConstIteratorFn(i, j) : 0; } QT_END_NAMESPACE diff --git a/src/gui/image/qppmhandler.cpp b/src/gui/image/qppmhandler.cpp index a0a1dcdaca9..8a413ded95e 100644 --- a/src/gui/image/qppmhandler.cpp +++ b/src/gui/image/qppmhandler.cpp @@ -123,8 +123,8 @@ static bool read_pbm_body(QIODevice *device, char type, int w, int h, int mcc, Q break; case '2': // ascii PGM case '5': // raw PGM - nbits = 8; - format = QImage::Format_Grayscale8; + nbits = mcc <= std::numeric_limits<uint8_t>::max() ? 8 : 16; + format = mcc <= std::numeric_limits<uint8_t>::max() ? QImage::Format_Grayscale8 : QImage::Format_Grayscale16; break; case '3': // ascii PPM case '6': // raw PPM @@ -175,20 +175,20 @@ static bool read_pbm_body(QIODevice *device, char type, int w, int h, int mcc, Q } } delete[] buf24; - } else if (nbits == 8 && mcc > 255) { // type 5 16bit - pbm_bpl = 2*w; + } else if (nbits == 16) { // type 5 16bit + pbm_bpl = sizeof(uint16_t)*w; uchar *buf16 = new uchar[pbm_bpl]; for (y=0; y<h; y++) { if (device->read((char *)buf16, pbm_bpl) != pbm_bpl) { delete[] buf16; return false; } - uchar *p = outImage->scanLine(y); - uchar *end = p + w; - uchar *b = buf16; + uint16_t *p = reinterpret_cast<uint16_t *>(outImage->scanLine(y)); + uint16_t *end = p + w; + uint16_t *b = reinterpret_cast<uint16_t *>(buf16); while (p < end) { - *p++ = (b[0] << 8 | b[1]) * 255 / mcc; - b += 2; + *p++ = qFromBigEndian(*b) * std::numeric_limits<uint16_t>::max() / mcc; + b++; } } delete[] buf16; @@ -225,13 +225,25 @@ static bool read_pbm_body(QIODevice *device, char type, int w, int h, int mcc, Q *p++ = b; } } else if (nbits == 8) { - if (mcc == 255) { + if (mcc == std::numeric_limits<uint8_t>::max()) { while (n-- && ok) { *p++ = read_pbm_int(device, &ok); } } else { while (n-- && ok) { - *p++ = (read_pbm_int(device, &ok) & 0xffff) * 255 / mcc; + *p++ = (read_pbm_int(device, &ok) & 0xffff) * std::numeric_limits<uint8_t>::max() / mcc; + } + } + } else if (nbits == 16) { + uint16_t* data = reinterpret_cast<uint16_t*>(p); + qsizetype numPixel = n/2; + if (mcc == std::numeric_limits<uint16_t>::max()) { + while (numPixel-- && ok) { + *data++ = read_pbm_int(device, &ok); + } + } else { + while (numPixel-- && ok) { + *data++ = (read_pbm_int(device, &ok) & 0xffff) * std::numeric_limits<uint16_t>::max() / mcc; } } } else { // 32 bits @@ -280,7 +292,7 @@ static bool write_pbm_image(QIODevice *out, const QImage &sourceImage, QByteArra if (format == "pbm") { image = image.convertToFormat(QImage::Format_Mono); } else if (gray) { - image = image.convertToFormat(QImage::Format_Grayscale8); + image = image.depth() <= 8 ? image.convertToFormat(QImage::Format_Grayscale8) : image.convertToFormat(QImage::Format_Grayscale16); } else { switch (image.format()) { case QImage::Format_Mono: @@ -388,6 +400,34 @@ static bool write_pbm_image(QIODevice *out, const QImage &sourceImage, QByteArra delete [] buf; break; } + case 16: { + str.insert(1, gray ? '5' : '6'); + str.append("65535\n"); + if (out->write(str, str.size()) != str.size()) + return false; + qsizetype bpl = sizeof(uint16_t) * qsizetype(w) * (gray ? 1 : 3); + uchar *buf = new uchar[bpl]; + for (uint y=0; y<h; y++) { + const uint16_t *b = reinterpret_cast<const uint16_t *>(image.constScanLine(y)); + uint16_t *p = reinterpret_cast<uint16_t *>(buf); + uint16_t *end = reinterpret_cast<uint16_t *>(buf + bpl); + if (gray) { + while (p < end) + *p++ = qToBigEndian(*b++); + } else { + while (p < end) { + uchar color = qToBigEndian(*b++); + *p++ = color; + *p++ = color; + *p++ = color; + } + } + if (bpl != (qsizetype)out->write((char*)buf, bpl)) + return false; + } + delete [] buf; + break; + } case 32: { str.insert(1, '6'); @@ -530,7 +570,10 @@ QVariant QPpmHandler::option(ImageOption option) const break; case '2': // ascii PGM case '5': // raw PGM - format = QImage::Format_Grayscale8; + if (mcc <= std::numeric_limits<uint8_t>::max()) + format = QImage::Format_Grayscale8; + else + format = QImage::Format_Grayscale16; break; case '3': // ascii PPM case '6': // raw PPM diff --git a/src/network/access/http2/http2protocol.cpp b/src/network/access/http2/http2protocol.cpp index 050beacb31c..314be07f952 100644 --- a/src/network/access/http2/http2protocol.cpp +++ b/src/network/access/http2/http2protocol.cpp @@ -188,8 +188,9 @@ bool is_protocol_upgraded(const QHttpNetworkReply &reply) if (reply.statusCode() != 101) return false; + const auto values = reply.header().values(QHttpHeaders::WellKnownHeader::Upgrade); // Do some minimal checks here - we expect 'Upgrade: h2c' to be found. - for (const auto &v : reply.header().values(QHttpHeaders::WellKnownHeader::Upgrade)) { + for (const auto &v : values) { if (v.compare("h2c", Qt::CaseInsensitive) == 0) return true; } diff --git a/src/network/access/qhttp2connection.cpp b/src/network/access/qhttp2connection.cpp index eda1112fe17..2895e8335d2 100644 --- a/src/network/access/qhttp2connection.cpp +++ b/src/network/access/qhttp2connection.cpp @@ -454,7 +454,7 @@ void QHttp2Stream::internalSendDATA() "[%p] stream %u, exhausted device %p, sent END_STREAM? %d, %ssending end stream " "after DATA", connection, m_streamID, m_uploadByteDevice, sentEND_STREAM, - m_endStreamAfterDATA ? "" : "not "); + !sentEND_STREAM && m_endStreamAfterDATA ? "" : "not "); if (!sentEND_STREAM && m_endStreamAfterDATA) { // We need to send an empty DATA frame with END_STREAM since we // have exhausted the device, but we haven't sent END_STREAM yet. @@ -690,8 +690,9 @@ void QHttp2Stream::handleDATA(const Frame &inboundFrame) m_recvWindow -= qint32(inboundFrame.payloadSize()); const bool endStream = inboundFrame.flags().testFlag(FrameFlag::END_STREAM); + const bool ignoreData = connection->streamIsIgnored(m_streamID); // Uncompress data if needed and append it ... - if (inboundFrame.dataSize() > 0 || endStream) { + if ((inboundFrame.dataSize() > 0 || endStream) && !ignoreData) { QByteArray fragment(reinterpret_cast<const char *>(inboundFrame.dataBegin()), inboundFrame.dataSize()); if (endStream) @@ -1245,16 +1246,12 @@ void QHttp2Connection::connectionError(Http2Error errorCode, const char *message { Q_ASSERT(message); // RFC 9113, 6.8: An endpoint MAY send multiple GOAWAY frames if circumstances change. - // Anyway, we do not send multiple GOAWAY frames. - if (m_goingAway) - return; qCCritical(qHttp2ConnectionLog, "[%p] Connection error: %s (%d)", this, message, int(errorCode)); // RFC 9113, 6.8: Endpoints SHOULD always send a GOAWAY frame before closing a connection so // that the remote peer can know whether a stream has been partially processed or not. - m_goingAway = true; sendGOAWAY(errorCode); auto messageView = QLatin1StringView(message); @@ -1295,6 +1292,20 @@ bool QHttp2Connection::isInvalidStream(quint32 streamID) noexcept return (!stream || stream->wasResetbyPeer()) && !streamWasResetLocally(streamID); } +/*! + When we send a GOAWAY we also send the ID of the last stream we know about + at the time. Any stream that starts after this one is ignored, but we still + have to process HEADERS due to compression state, and DATA due to stream and + connection window size changes. + Other than that - any \a streamID for which this returns true should be + ignored, and deleted at the earliest convenience. +*/ +bool QHttp2Connection::streamIsIgnored(quint32 streamID) const noexcept +{ + const bool streamIsRemote = (streamID & 1) == (m_connectionType == Type::Client ? 0 : 1); + return Q_UNLIKELY(streamIsRemote && m_lastStreamToProcess < streamID); +} + bool QHttp2Connection::sendClientPreface() { QIODevice *socket = getSocket(); @@ -1359,9 +1370,16 @@ bool QHttp2Connection::sendWINDOW_UPDATE(quint32 streamID, quint32 delta) bool QHttp2Connection::sendGOAWAY(Http2::Http2Error errorCode) { + m_goingAway = true; + // If this is the first time, start the timer: + if (m_lastStreamToProcess == Http2::lastValidStreamID) + m_goawayGraceTimer.setRemainingTime(GoawayGracePeriod); + m_lastStreamToProcess = std::min(m_lastIncomingStreamID, m_lastStreamToProcess); + qCDebug(qHttp2ConnectionLog, "[%p] Sending GOAWAY frame, error code %u, last stream %u", this, + errorCode, m_lastStreamToProcess); frameWriter.start(FrameType::GOAWAY, FrameFlag::EMPTY, Http2PredefinedParameters::connectionStreamID); - frameWriter.append(quint32(m_lastIncomingStreamID)); + frameWriter.append(m_lastStreamToProcess); frameWriter.append(quint32(errorCode)); return frameWriter.write(*getSocket()); } @@ -1411,8 +1429,20 @@ void QHttp2Connection::handleDATA() if (stream) stream->handleDATA(inboundFrame); - if (inboundFrame.flags().testFlag(FrameFlag::END_STREAM)) - emit receivedEND_STREAM(streamID); + + if (inboundFrame.flags().testFlag(FrameFlag::END_STREAM)) { + const bool ignoreData = stream && streamIsIgnored(stream->streamID()); + if (!ignoreData) { + emit receivedEND_STREAM(streamID); + } else { + // Stream opened after our GOAWAY cut-off. We would just drop the + // data, but needed to handle it enough to track sizes of streams and + // connection windows. Since we've now taken care of that, we can + // at last close and delete it. + stream->setState(QHttp2Stream::State::Closed); + delete stream; + } + } if (sessionReceiveWindowSize < maxSessionReceiveWindowSize / 2) { // @future[consider]: emit signal instead @@ -1454,8 +1484,15 @@ void QHttp2Connection::handleHEADERS() return; } - qCDebug(qHttp2ConnectionLog, "[%p] Created new incoming stream %d", this, streamID); - emit newIncomingStream(newStream); + qCDebug(qHttp2ConnectionLog, "[%p] New incoming stream %d", this, streamID); + if (!streamIsIgnored(newStream->streamID())) { + emit newIncomingStream(newStream); + } else if (m_goawayGraceTimer.hasExpired()) { + // We gave the peer some time to handle the GOAWAY message, but they have started a new + // stream, so we error out. + connectionError(Http2Error::PROTOCOL_ERROR, "Peer refused to GOAWAY."); + return; + } } else if (streamWasResetLocally(streamID)) { qCDebug(qHttp2ConnectionLog, "[%p] Received HEADERS on previously locally reset stream %d (must process but ignore)", @@ -1500,6 +1537,9 @@ void QHttp2Connection::handlePRIORITY() || inboundFrame.type() == FrameType::HEADERS); const auto streamID = inboundFrame.streamID(); + if (streamIsIgnored(streamID)) + return; + // RFC 9913, 6.3: If a PRIORITY frame is received with a stream identifier of 0x00, the // recipient MUST respond with a connection error if (streamID == connectionStreamID) @@ -1534,11 +1574,14 @@ void QHttp2Connection::handleRST_STREAM() { Q_ASSERT(inboundFrame.type() == FrameType::RST_STREAM); + const auto streamID = inboundFrame.streamID(); + if (streamIsIgnored(streamID)) + return; + // RFC 9113, 6.4: RST_STREAM frames MUST be associated with a stream. // If a RST_STREAM frame is received with a stream identifier of 0x0, // the recipient MUST treat this as a connection error (Section 5.4.1) // of type PROTOCOL_ERROR. - const auto streamID = inboundFrame.streamID(); if (streamID == connectionStreamID) return connectionError(PROTOCOL_ERROR, "RST_STREAM on 0x0"); @@ -1765,31 +1808,32 @@ void QHttp2Connection::handleGOAWAY() const uchar *const src = inboundFrame.dataBegin(); // RFC 9113, 4.1: 31-bit Stream ID; lastValidStreamID(0x7FFFFFFF) masks out the reserved MSB - quint32 lastStreamID = qFromBigEndian<quint32>(src) & lastValidStreamID; + const quint32 lastStreamID = qFromBigEndian<quint32>(src) & lastValidStreamID; const Http2Error errorCode = Http2Error(qFromBigEndian<quint32>(src + 4)); - if (!lastStreamID) { - // "The last stream identifier can be set to 0 if no - // streams were processed." - lastStreamID = 1; - } else if (!(lastStreamID & 0x1)) { - // 5.1.1 - we (client) use only odd numbers as stream identifiers. + // 6.8 "the GOAWAY contains the stream identifier of the last peer-initiated stream that was + // or might be processed on the sending endpoint in this connection." + // Alternatively, they can specify 0 as the last stream ID, meaning they are not intending to + // process any remaining stream(s). + const quint32 LocalMask = m_connectionType == Type::Client ? 1 : 0; + // The stream must match the LocalMask, meaning we initiated it, for the last stream ID to make + // sense - they are not processing their own streams. + if (lastStreamID != 0 && (lastStreamID & 0x1) != LocalMask) return connectionError(PROTOCOL_ERROR, "GOAWAY with invalid last stream ID"); - } else if (lastStreamID >= m_nextStreamID) { - // "A server that is attempting to gracefully shut down a connection SHOULD - // send an initial GOAWAY frame with the last stream identifier set to 2^31-1 - // and a NO_ERROR code." - if (lastStreamID != lastValidStreamID || errorCode != HTTP2_NO_ERROR) - return connectionError(PROTOCOL_ERROR, "GOAWAY invalid stream/error code"); - } else { - lastStreamID += 2; - } + qCDebug(qHttp2ConnectionLog, "[%p] Received GOAWAY frame, error code %u, last stream %u", + this, errorCode, lastStreamID); m_goingAway = true; emit receivedGOAWAY(errorCode, lastStreamID); - for (quint32 id = lastStreamID; id < m_nextStreamID; id += 2) { + // Since the embedded stream ID is the last one that was or _might be_ processed, + // we cancel anything that comes after it. 0 can be used in the special case that + // no streams at all were or will be processed. + const quint32 firstPossibleStream = m_connectionType == Type::Client ? 1 : 2; + const quint32 firstCancelledStream = lastStreamID ? lastStreamID + 2 : firstPossibleStream; + Q_ASSERT((firstCancelledStream & 0x1) == LocalMask); + for (quint32 id = firstCancelledStream; id < m_nextStreamID; id += 2) { QHttp2Stream *stream = m_streams.value(id, nullptr); if (stream && stream->isActive()) stream->finishWithError(errorCode, "Received GOAWAY"_L1); @@ -1810,7 +1854,8 @@ void QHttp2Connection::handleWINDOW_UPDATE() // errors on the connection flow-control window MUST be treated as a connection error const bool valid = delta && delta <= quint32(std::numeric_limits<qint32>::max()); const auto streamID = inboundFrame.streamID(); - + if (streamIsIgnored(streamID)) + return; // RFC 9113, 6.9: A WINDOW_UPDATE frame with a length other than 4 octets MUST be treated // as a connection error (Section 5.4.1) of type FRAME_SIZE_ERROR. @@ -1940,6 +1985,18 @@ void QHttp2Connection::handleContinuedHEADERS() if (streamWasResetLocally(streamID) || streamIt == m_streams.cend()) return; // No more processing without a stream from here on. + if (streamIsIgnored(streamID)) { + // Stream was established after GOAWAY cut-off, we ignore it, but we + // have to process things that alter state. That already happened, so we + // stop here. + if (continuedFrames[0].flags().testFlag(Http2::FrameFlag::END_STREAM)) { + if (QHttp2Stream *stream = streamIt.value()) { + stream->setState(QHttp2Stream::State::Closed); + delete stream; + } + } + return; + } switch (firstFrameType) { case FrameType::HEADERS: diff --git a/src/network/access/qhttp2connection_p.h b/src/network/access/qhttp2connection_p.h index dcdc0f91318..f3f14145278 100644 --- a/src/network/access/qhttp2connection_p.h +++ b/src/network/access/qhttp2connection_p.h @@ -283,6 +283,8 @@ private: bool isInvalidStream(quint32 streamID) noexcept; bool streamWasResetLocally(quint32 streamID) noexcept; + Q_ALWAYS_INLINE + bool streamIsIgnored(quint32 streamID) const noexcept; void connectionError(Http2::Http2Error errorCode, const char *message); // Connection failed to be established? @@ -400,6 +402,10 @@ private: bool m_goingAway = false; bool pushPromiseEnabled = false; quint32 m_lastIncomingStreamID = Http2::connectionStreamID; + // Gets lowered when/if we send GOAWAY: + quint32 m_lastStreamToProcess = Http2::lastValidStreamID; + static constexpr std::chrono::duration GoawayGracePeriod = std::chrono::seconds(60); + QDeadlineTimer m_goawayGraceTimer; bool m_prefaceSent = false; diff --git a/src/network/access/qhttpnetworkheader.cpp b/src/network/access/qhttpnetworkheader.cpp index acb4fcfa8e1..d09d856e441 100644 --- a/src/network/access/qhttpnetworkheader.cpp +++ b/src/network/access/qhttpnetworkheader.cpp @@ -4,8 +4,6 @@ #include "qhttpnetworkheader_p.h" -#include <algorithm> - QT_BEGIN_NAMESPACE QHttpNetworkHeader::~QHttpNetworkHeader() diff --git a/src/network/access/qnetworkrequest.cpp b/src/network/access/qnetworkrequest.cpp index 87f113be5dc..5047fc77bd5 100644 --- a/src/network/access/qnetworkrequest.cpp +++ b/src/network/access/qnetworkrequest.cpp @@ -21,7 +21,6 @@ #include "QtCore/private/qduplicatetracker_p.h" #include "QtCore/private/qtools_p.h" -#include <ctype.h> #if QT_CONFIG(datestring) # include <stdio.h> #endif diff --git a/src/plugins/platforms/wayland/qwaylandinputcontext.cpp b/src/plugins/platforms/wayland/qwaylandinputcontext.cpp index 5ab285ad97d..0ccc4dba57a 100644 --- a/src/plugins/platforms/wayland/qwaylandinputcontext.cpp +++ b/src/plugins/platforms/wayland/qwaylandinputcontext.cpp @@ -192,12 +192,10 @@ void QWaylandInputContext::setFocusObject(QObject *object) if (window && window->handle()) { if (mCurrentWindow.data() != window) { if (!inputMethodAccepted()) { - if (mCurrentWindow) { - auto *surface = static_cast<QWaylandWindow *>(mCurrentWindow->handle())->wlSurface(); - if (surface) - inputInterface->disableSurface(surface); - mCurrentWindow.clear(); - } + auto *surface = static_cast<QWaylandWindow *>(window->handle())->wlSurface(); + if (surface) + inputInterface->disableSurface(surface); + mCurrentWindow.clear(); } else { auto *surface = static_cast<QWaylandWindow *>(window->handle())->wlSurface(); if (surface) { diff --git a/src/plugins/platforms/windows/qwindowswindowclassdescription.cpp b/src/plugins/platforms/windows/qwindowswindowclassdescription.cpp index 63e16260b62..e2e46a7b215 100644 --- a/src/plugins/platforms/windows/qwindowswindowclassdescription.cpp +++ b/src/plugins/platforms/windows/qwindowswindowclassdescription.cpp @@ -75,4 +75,14 @@ QWindowsWindowClassDescription QWindowsWindowClassDescription::fromWindow(const return description; } +QDebug operator<<(QDebug dbg, const QWindowsWindowClassDescription &description) +{ + dbg << description.name + << " style=0x" << Qt::hex << description.style << Qt::dec + << " brush=" << description.brush + << " hasIcon=" << description.hasIcon; + + return dbg; +} + QT_END_NAMESPACE diff --git a/src/plugins/platforms/windows/qwindowswindowclassdescription.h b/src/plugins/platforms/windows/qwindowswindowclassdescription.h index 9423abf9d2d..3acca65b8a2 100644 --- a/src/plugins/platforms/windows/qwindowswindowclassdescription.h +++ b/src/plugins/platforms/windows/qwindowswindowclassdescription.h @@ -23,6 +23,9 @@ struct QWindowsWindowClassDescription HBRUSH brush{ nullptr }; bool hasIcon{ false }; bool shouldAddPrefix{ true }; + +private: + friend QDebug operator<<(QDebug dbg, const QWindowsWindowClassDescription &description); }; QT_END_NAMESPACE diff --git a/src/plugins/platforms/windows/qwindowswindowclassregistry.cpp b/src/plugins/platforms/windows/qwindowswindowclassregistry.cpp index c330720a09c..9c9ffcfeefa 100644 --- a/src/plugins/platforms/windows/qwindowswindowclassregistry.cpp +++ b/src/plugins/platforms/windows/qwindowswindowclassregistry.cpp @@ -114,9 +114,9 @@ QString QWindowsWindowClassRegistry::registerWindowClass(const QWindowsWindowCla qPrintable(className)); m_registeredWindowClassNames.insert(className); - qCDebug(lcQpaWindowClass).nospace() << __FUNCTION__ << ' ' << className - << " style=0x" << Qt::hex << description.style << Qt::dec - << " brush=" << description.brush << " icon=" << description.hasIcon << " atom=" << atom; + + qCDebug(lcQpaWindowClass).nospace() << __FUNCTION__ << ' ' << className << ' ' << description << " atom=" << atom; + return className; } diff --git a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp index 9be046c75be..b05a055252b 100644 --- a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp +++ b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp @@ -397,6 +397,7 @@ private slots: void iterateAssociativeContainerElements_data(); void iterateAssociativeContainerElements() { runTestFunction(); } void iterateContainerElements(); + void emptyContainerInterface(); void pairElements_data(); void pairElements() { runTestFunction(); } @@ -5324,6 +5325,26 @@ void tst_QVariant::iterateContainerElements() } } +void tst_QVariant::emptyContainerInterface() +{ + // An empty container interface should implicitly be of invalid size + // and its begin and end iterators should be equal. + + const QtMetaContainerPrivate::QMetaContainerInterface emptyContainerInterface {}; + QIterable emptyIterable(QMetaContainer(&emptyContainerInterface), nullptr); + + QCOMPARE(emptyIterable.size(), -1); + auto constBegin = emptyIterable.constBegin(); + auto constEnd = emptyIterable.constEnd(); + QVERIFY(constBegin == constEnd); + QCOMPARE(constEnd - constBegin, 0); + + auto mutableBegin = emptyIterable.mutableBegin(); + auto mutableEnd = emptyIterable.mutableEnd(); + QVERIFY(mutableBegin == mutableEnd); + QCOMPARE(mutableEnd - mutableBegin, 0); +} + template <typename Pair> static void testVariantPairElements() { QFETCH(std::function<void(void *)>, makeValue); diff --git a/tests/auto/gui/image/qimagereader/images/image16.pgm b/tests/auto/gui/image/qimagereader/images/image16.pgm new file mode 100644 index 00000000000..4e0b55131b0 --- /dev/null +++ b/tests/auto/gui/image/qimagereader/images/image16.pgm @@ -0,0 +1,260 @@ +P2 +16 +16 +65535 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +65535 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 +32767 diff --git a/tests/auto/gui/image/qimagereader/tst_qimagereader.cpp b/tests/auto/gui/image/qimagereader/tst_qimagereader.cpp index de9fea78ea6..8ccaf435f0b 100644 --- a/tests/auto/gui/image/qimagereader/tst_qimagereader.cpp +++ b/tests/auto/gui/image/qimagereader/tst_qimagereader.cpp @@ -610,6 +610,7 @@ void tst_QImageReader::imageFormat_data() QTest::newRow("pbm") << QString("image.pbm") << QByteArray("pbm") << QImage::Format_Mono; QTest::newRow("pgm") << QString("image.pgm") << QByteArray("pgm") << QImage::Format_Grayscale8; + QTest::newRow("pgm") << QString("image16.pgm") << QByteArray("pgm") << QImage::Format_Grayscale16; QTest::newRow("ppm-1") << QString("image.ppm") << QByteArray("ppm") << QImage::Format_RGB32; QTest::newRow("ppm-2") << QString("teapot.ppm") << QByteArray("ppm") << QImage::Format_RGB32; QTest::newRow("ppm-3") << QString("runners.ppm") << QByteArray("ppm") << QImage::Format_RGB32; diff --git a/tests/auto/network/access/qhttp2connection/tst_qhttp2connection.cpp b/tests/auto/network/access/qhttp2connection/tst_qhttp2connection.cpp index 8e8c90e14de..417655c31d9 100644 --- a/tests/auto/network/access/qhttp2connection/tst_qhttp2connection.cpp +++ b/tests/auto/network/access/qhttp2connection/tst_qhttp2connection.cpp @@ -8,6 +8,8 @@ #include <QtNetwork/private/hpack_p.h> #include <QtNetwork/private/bitstreams_p.h> +#include <QtCore/qregularexpression.h> + #include <limits> using namespace Qt::StringLiterals; @@ -35,6 +37,8 @@ private slots: void connectToServer(); void WINDOW_UPDATE(); void testCONTINUATIONFrame(); + void goaway_data(); + void goaway(); private: enum PeerType { Client, Server }; @@ -1051,6 +1055,112 @@ void tst_QHttp2Connection::testCONTINUATIONFrame() } } +void tst_QHttp2Connection::goaway_data() +{ + QTest::addColumn<bool>("endStreamOnHEADERS"); + QTest::addColumn<bool>("createNewStreamAfterDelay"); + QTest::addRow("end-on-headers") << true << false; + QTest::addRow("end-after-data") << false << false; + QTest::addRow("end-after-new-late-stream") << false << true; +} + +void tst_QHttp2Connection::goaway() +{ + QFETCH(const bool, endStreamOnHEADERS); + QFETCH(const bool, createNewStreamAfterDelay); + auto [client, server] = makeFakeConnectedSockets(); + auto connection = makeHttp2Connection(client.get(), {}, Client); + auto serverConnection = makeHttp2Connection(server.get(), {}, Server); + + QHttp2Stream *clientStream = connection->createStream().unwrap(); + QVERIFY(clientStream); + QVERIFY(waitForSettingsExchange(connection, serverConnection)); + + QSignalSpy newIncomingStreamSpy{ serverConnection, &QHttp2Connection::newIncomingStream }; + + QSignalSpy clientIncomingStreamSpy{ connection, &QHttp2Connection::newIncomingStream }; + QSignalSpy clientHeaderReceivedSpy{ clientStream, &QHttp2Stream::headersReceived }; + QSignalSpy clientGoawaySpy{ connection, &QHttp2Connection::receivedGOAWAY }; + + const HPack::HttpHeader headers = getRequiredHeaders(); + clientStream->sendHEADERS(headers, false); + + QVERIFY(newIncomingStreamSpy.wait()); + auto *serverStream = newIncomingStreamSpy.front().front().value<QHttp2Stream *>(); + QVERIFY(serverStream); + QVERIFY(serverConnection->sendGOAWAY(Http2::CANCEL)); + auto createStreamResult = serverConnection->createLocalStreamInternal(); + QVERIFY(createStreamResult.has_error()); + QCOMPARE(createStreamResult.error(), QHttp2Connection::CreateStreamError::ReceivedGOAWAY); + + QVERIFY(clientGoawaySpy.wait()); + QCOMPARE(clientGoawaySpy.size(), 1); + // The error code used: + QCOMPARE(clientGoawaySpy.first().first().value<Http2::Http2Error>(), Http2::CANCEL); + // Last ID that will be processed + QCOMPARE(clientGoawaySpy.first().last().value<quint32>(), clientStream->streamID()); + clientGoawaySpy.clear(); + + // Test that creating a stream the normal way results in an error: + QH2Expected<QHttp2Stream *, QHttp2Connection::CreateStreamError> + invalidStream = connection->createStream(); + QVERIFY(!invalidStream.ok()); + QVERIFY(invalidStream.has_error()); + QCOMPARE(invalidStream.error(), QHttp2Connection::CreateStreamError::ReceivedGOAWAY); + + // Directly create a stream to avoid the GOAWAY check: + quint32 nextStreamId = clientStream->streamID() + 2; + QHttp2Stream *secondClientStream = connection->createStreamInternal_impl(nextStreamId); + QSignalSpy streamResetSpy{ secondClientStream, &QHttp2Stream::rstFrameReceived }; + secondClientStream->sendHEADERS(headers, endStreamOnHEADERS); + // The stream should be ignored: + using namespace std::chrono_literals; + QVERIFY(!streamResetSpy.wait(100ms)); // We don't get reset because we are ignored + if (endStreamOnHEADERS) + return; + + secondClientStream->sendDATA("my data", createNewStreamAfterDelay); + // We cheat and try to send data after the END_STREAM flag has been sent + if (!createNewStreamAfterDelay) { + // Manually send a frame with END_STREAM so the QHttp2Stream thinks it's fine to send more + // DATA + connection->frameWriter.start(Http2::FrameType::DATA, Http2::FrameFlag::END_STREAM, + secondClientStream->streamID()); + connection->frameWriter.write(*connection->getSocket()); + QVERIFY(!streamResetSpy.wait(100ms)); // We don't get reset because we are ignored + + // Even without the GOAWAY this should fail (more activity after END_STREAM) + secondClientStream->sendDATA("my data", true); + QTest::ignoreMessage(QtCriticalMsg, + QRegularExpression(u".*Connection error: DATA on invalid stream.*"_s)); + QVERIFY(clientGoawaySpy.wait()); + QCOMPARE(clientGoawaySpy.size(), 1); + QCOMPARE(clientGoawaySpy.first().first().value<Http2::Http2Error>(), + Http2::ENHANCE_YOUR_CALM); + QCOMPARE(clientGoawaySpy.first().last().value<quint32>(), clientStream->streamID()); + return; // connection is dead by now + } + + // Override the deadline timer so we don't have to wait too long + serverConnection->m_goawayGraceTimer.setRemainingTime(50ms); + + // We can create the stream whenever, it is not noticed by the server until we send something. + nextStreamId += 2; + QHttp2Stream *rejectedStream = connection->createStreamInternal_impl(nextStreamId); + // Sleep until the grace period is over: + QTRY_VERIFY(serverConnection->m_goawayGraceTimer.hasExpired()); + + QVERIFY(rejectedStream->sendHEADERS(headers, true)); + + QTest::ignoreMessage(QtCriticalMsg, + QRegularExpression(u".*Connection error: Peer refused to GOAWAY\\..*"_s)); + QVERIFY(clientGoawaySpy.wait()); + QCOMPARE(clientGoawaySpy.size(), 1); + QCOMPARE(clientGoawaySpy.first().first().value<Http2::Http2Error>(), Http2::PROTOCOL_ERROR); + // The first stream is still the last processed one: + QCOMPARE(clientGoawaySpy.first().last().value<quint32>(), clientStream->streamID()); +} + QTEST_MAIN(tst_QHttp2Connection) #include "tst_qhttp2connection.moc" |