Contribute to Open Source. Search issue labels to find the right project for you!

not_null<shared_ptr> overhead

Microsoft/GSL

What is the intended usage of not_null with std::shared_ptr? There is a significant overhead to wrapping std::shared_ptr in not_null because every access results in a costly copy of the shared_ptr. As not_null cannot be used with unique_ptr, it seems that not_null is not really usable with smart pointers in general.

Updated 17/08/2017 01:47

string less struct

Microsoft/GSL

When using a structure like std::map with string types as a key (std::string, gsl::cstring_span, gsl::string_span, etc.), the default std::less comparison cannot compare similar string types. It would be nice to have an implementation of a string comparison object that works with standard strings and gsl string spans.

Problem example: ``` std::string str = “some string”; gsl::cstring_span<> cstrSpan = str; gsl::string_span<> strSpan = str;

std::map<std::string, int> strMap; strMap.find(cstrSpan); //error strMap.find(strSpan); //error strMap.find(str);

std::map<gsl::string_span<>, int> strSpanMap; strSpanMap.find(cstrSpan); //error strSpanMap.find(strSpan); strSpanMap.find(str);

std::map<gsl::cstring_span<>, int> cstrSpanMap; cstrSpanMap.find(cstrSpan); cstrSpanMap.find(strSpan); cstrSpanMap.find(str); ```

Implementation example: ``` struct string_less { using is_transparent = void;

template <class CharT, class CharTTraits, class AllocatorT>
bool operator()(const std::basic_string<CharT, CharTTraits, AllocatorT> &lhs, const std::basic_string<CharT, CharTTraits, AllocatorT> &rhs) const {
    return lhs < rhs;
}

template <class CharT, ptrdiff_t Extent, class CharTTraits, class AllocatorT>
bool operator()(const std::basic_string<CharT, CharTTraits, AllocatorT> &lhs, const gsl::basic_string_span<const CharT, Extent> &rhs) const {
    return lhs < rhs;
}

template <class CharT, ptrdiff_t Extent, class CharTTraits, class AllocatorT>
bool operator()(const std::basic_string<CharT, CharTTraits, AllocatorT> &lhs, const gsl::basic_string_span<CharT, Extent> &rhs) const {
    return lhs < rhs;
}

template <class CharT, ptrdiff_t Extent, class CharTTraits, class AllocatorT>
bool operator()(const gsl::basic_string_span<const CharT, Extent> &lhs, const std::basic_string<CharT, CharTTraits, AllocatorT> &rhs) const {
    return lhs < rhs;
}

template <class CharT, ptrdiff_t Extent, class CharTTraits, class AllocatorT>
bool operator()(const gsl::basic_string_span<CharT, Extent> &lhs, const std::basic_string<CharT, CharTTraits, AllocatorT> &rhs) const {
    return lhs < rhs;
}

template <class CharT, ptrdiff_t Extent>
bool operator()(const gsl::basic_string_span<const CharT, Extent> &lhs, const gsl::basic_string_span<const CharT, Extent> &rhs) const {
    return lhs < rhs;
}

template <class CharT, ptrdiff_t Extent>
bool operator()(const gsl::basic_string_span<CharT, Extent> &lhs, const gsl::basic_string_span<CharT, Extent> &rhs) const {
    return lhs < rhs;
}

template <class CharT, ptrdiff_t Extent>
bool operator()(const gsl::basic_string_span<const CharT, Extent> &lhs, const gsl::basic_string_span<CharT, Extent> &rhs) const {
    return lhs < rhs;
}

template <class CharT, ptrdiff_t Extent>
bool operator()(const gsl::basic_string_span<CharT, Extent> &lhs, const gsl::basic_string_span<const CharT, Extent> &rhs) const {
    return lhs < rhs;
}

}; ```

Updated 17/08/2017 01:46

Game Domino QQ - Chip của Member bet trên bàn không chạy các effect tương tác với Member.

nguyenphuquang/cardgame

Case như sau: 2 member test: VN-test04 và VN-test99. - Step1: sau 1 vòng 2 member bet. => kết thúc: Effect chip đã bet chạy từ 2 member vào giữa bàn. Hiện tại Web không có hiển thị effect này. - Step2: sau khi 2 member click chọn OK trên popup Confirm. => End game: web không hiển thị các pot và effect chip bay về member win, hiện tại các pot và effect chỉ hiển thị chip bay về dealer.

2

Updated 07/08/2017 08:32

Decorate function declarations to support their use in CUDA

Microsoft/GSL

Most of the GSL code is theoretically and practically usable when writing CUDA C++ code which runs on a GPU. But a function needs to be designated for such use. When compiling C++ as CUDA code, a function is by default CPU-only, having an implicit __host__ designator; marking it __device__ makes it on-GPU only, and marking it __host__ __device__ means dual use (two separate compilations).

GSL Lite already has decorations for CUDA support; would you consider adding them as well? Basically, it’s just a macro which, when not compiling CUDA, expands to nothing. Here’s what gsl-lite has:

#ifndef   gsl_api
# ifdef   __CUDACC__
#  define gsl_api __host__ __device__
# else
#  define gsl_api /*gsl_api*/
# endif
#endif
Updated 17/08/2017 01:47

Creating string_spans with `char16_t` & `char32_t` is not working.

Microsoft/GSL

The compiler complains when you try to make a span with char16_t & char32_t cpp gsl::basic_string_span<const char> c("stuff"); // good gsl::basic_string_span<const wchar_t> wc(L"stuff"); // good gsl::basic_string_span<const char16_t> u16c(u"stuff"); // compile error! gsl::basic_string_span<const char32_t> u32c(U"stuff"); // compile error! On inspection this appears to be because various overloads and functor specializations are missing from <gsl/string_span>.

Updated 17/08/2017 01:58

Creating a Span of length 0 is ambiguous

Microsoft/GSL

The following code is ambiguous:

uint32_t a[] = {1, 2, 3};
span<uint32_t> s(a, 0);

Since the literal 0 is also a pointer, the compiler can’t disambiguate between the pointer and length constructor and the two-pointer constructor. The same issue exists, at least in MSVC 2015, even with expressions evaluating to 0 such as +0 and 1-1.

A zero-length span over an existing range is something reasonable to want, semantically. One possible workaround might be to avoid the literal 0:

uint32_t a[] = {1, 2, 3};
span<uint32_t> s(a, std::ptrdiff_t{});

However this is clunky at best. There are two solutions I considered here

First: template <typename Pointer, typename std::enable_if_t<std::is_constructible_v<pointer, Pointer>, int> = 0> span(Pointer lastElem, Pointer lastElem);

This makes the above example correctly create a 0-length Span pointing to the array. It does, however make span<int> s(nullptr, nullptr) fail to compile, since std::distance doesn’t work on nullptr_t.

Another solution is

template <typename = void>
span(pointer firstElem, pointer lastElem);

This simply ranks the (now template) two-pointer constructor lower than the pointer-length constructor.

Updated 17/08/2017 01:50 4 Comments

string_span static array constexpr constructor not actually constexpr

Microsoft/GSL

When trying to make a constexpr string_span from a static array, like so:

constexpr gsl::string_span<> str {"asdf"};

VS 2017 gives the errors: error C2131: expression did not evaluate to a constant note: failure was caused by call of undefined function or one not declared 'constexpr' note: see usage of 'gsl::basic_string_span<const char,-1>::remove_z' note: while evaluating 'gsl::basic_string_span<const char,-1>::basic_string_span(&span, &{97,115,100,102,0})' error C2131: expression did not evaluate to a constant note: failure was caused by call of undefined function or one not declared 'constexpr' note: see usage of 'gsl::basic_string_span<const char,-1>::remove_z'

The constructor called here says: // From static arrays - if 0-terminated, remove 0 from the view // All other containers allow 0s within the length, so we do not remove them template <std::size_t N> constexpr basic_string_span(element_type (&arr)[N]) : span_(remove_z(arr)) { }

Since all we really want to do is remove the trailing zero from string literals, it seems like a simpler constructor like so might do what we want and allow for constexpr string spans: // From static arrays - if 0-terminated, remove 0 from the view // All other containers allow 0s within the length, so we do not remove them template <std::size_t N> constexpr basic_string_span(element_type (&arr)[N]) : span_(arr, arr[N - 1] ? N : N - 1) { } remove_z could also be fixed to be correctly constexpr, I suppose.

The comment seems to imply that we don’t care about embedded nulls, so we don’t need to look for the first null on the string, just remove the last one if there is one (string literals always have one). If someone hard-coded a static char array with nulls in it, they’d just have to be aware that a single ending null would be removed, but that’s no worse than the current situation.

Updated 17/08/2017 01:58

cppcon2016 gsl talk speaks of tags,releases, milestones

Microsoft/GSL

Its not clear to me how to tell if the daily commits have accumulated enough goodness that I should download a new “version” and upgrade.

Last time I took a snapshot you hadn’t moved to Catch…now you have. Plus how do I tell what is shipping with VS2017 15.2 compared to the github?

Thanks!

Updated 25/07/2017 05:51 4 Comments

Game Poker - Trường hợp Card trên tay member win không show, hiện tại chương trình xử lý lỗi =>Bug làm show card.

nguyenphuquang/cardgame

Điều kiện: 1. Mở trình duyệt Firefox, mở 2 tab và đăng nhập vào 2 member load vào game Poker. 2. 2 member start cùng 1 game. 3. 2 member bet đến khi Dealer chia 3 cards trên bàn. 4. 1 trong 2 member thực hiện act click “Fold”.

Bug: End game, ở màn hình Member lose, thấy card trên tay member win show. Yêu cầu: Card trên tay không show. 09

Updated 24/07/2017 03:04

Validate select input with chosen script

victorjonsson/jQuery-Form-Validator

When we have a select with chosen script that hide the select input, the validation feedback don’t appear. Is there a way to fix or solve it?

I have also set the option: validateHiddenInputs: true that’s works, in fact the assign of css class “has-success” works fine but don’t appear the green icon beacuse the select is hidden.

Updated 20/07/2017 11:03 2 Comments

Game Poker - Browser Firefox - Chuyển tab, 2 card trên tay member hiển thị sai trạng thái.

nguyenphuquang/cardgame

Dùng 2 member test cho trường hợp này. - Bắt đầu game1: Member VN-test07 và VN-test04, VN-test07 click Fold hoặc trong thời gian bet màn hình đang hiển thị cho member VN-test07, Chúng ta chuyển tab sang nàm hình của member VN-test04 và chờ timeline chạy hết (VN-test07 fold) Kết thúc game1. - Bắt đầu game2: Màn hình đang hiển thị member VN-test04, chuyển tab sang màn hình member VN-test07, xuất hiện bug như ảnh bên dưới. 07

Updated 19/07/2017 07:20

Remove span ctors that take unique_ptr and shared_ptr

Microsoft/GSL

@CaseyCarter pointed out to me that these two constructors might seem useful (because they save some typing)…but at scale will lead to implicit conversions where they might not be desirable. I believe the Ranges library has some related experience in this space that informs this opinion.

As I am keen to avoid repeating the mistakes of others, this issue serves as notice that I intend to remove these two constructors soon.

Updated 11/07/2017 20:31

Games - Browser Firefox - Chuyển tab - Những bất cập về thời gian (Phần 1).

nguyenphuquang/cardgame

Game Ceme: khi chuyển tab, các vòng timeline luôn ở vị trí start, nhưng thực tế đã bị trể. 05

Cũng trong game Ceme, với 2 User cùng start game. Step1: Người chơi chuyển tab nhanh qua màn hình game đến đúng time popup mở card xuất hiện. Step2: Chuyển tab sang 1 member khác và chờ đến khi start ván sau Step3: Chuyển tab về, nhận thấy Popup vẫn hiển thị trạng thái như game trước. 04

Game Domino QQ cũng bị trường hợp tương tự với Popup Confirm. Thực hiện như 3 bước trên, kế quả popup vẫn sẽ còn hiển thị và đếm tiếp số giây bên trong. 03

Updated 13/07/2017 09:00 1 Comments

cant validate using Event

victorjonsson/jQuery-Form-Validator

whats wrong with my code, thanks for any suggest

<script>
$.validate({
    form:'#formSemester',
    validateOnEvent:true
});

var errors = [],

// Validation configuration
conf = {
  onElementValidate : function(valid, $el, $form, errorMess) {
     if( !valid ) {
      // gather up the failed validations
      errors.push({el: $el, error: errorMess});
     }
  }
},
lang = 'en'
;


$.formUtils.loadModules('security, date');

$('#Ctombol').on('click', function() {
   // reset error array
   errors = [];
   if( !$(this).isValid(lang, conf, false) ) {
    displayErrors(errors);
   } else {
   // The form is valid
   $('#konfirS').modal('show');
   }
});

</script>
Updated 03/07/2017 09:21 1 Comments

GSL tests broken with GCC 7.1.0

Microsoft/GSL
COLLECT_LTO_WRAPPER=/mnt/fscratch0/pub/build/root-gcc-7.1.0-system/libexec/gcc/x86_64-pc-linux-gnu/7.1.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../../dependencies/gcc-7.1.0/configure --prefix=/mnt/fscratch0/pub/build/root-gcc-7.1.0-system --enable-languages=c,c++ --without-multilib
Thread model: posix
gcc version 7.1.0 (GCC)

This produces a ton of sign error spam when compiling tests.

Example:

                 from /mnt/devdiskvault/gfurnish/ap/GSL/tests/algorithm_tests.cpp:19:
/mnt/devdiskvault/gfurnish/ap/GSL/include/gsl/span: In constructor ‘constexpr gsl::span<ElementType, Extent>::span(gsl::span<ElementType, Extent>::element_type (&)[N])’:
/mnt/devdiskvault/gfurnish/ap/GSL/include/gsl/span:373:51: error: conversion to ‘long int’ from ‘long unsigned int’ may change the sign of the result [-Werror=sign-conversion]
         : storage_(&arr[0], details::extent_type<N>())

See log I haven’t looked into why this is happening yet but removing the -Wsign-conversion warning from tests does fix the issue.

Updated 13/07/2017 20:25 7 Comments

not_null comparison

Microsoft/GSL

This code causes compiler error C2678 in Visual Studio 2017. Why is that?

void test()
{
    struct A {};
    struct B : public A {} b;

    const B* pB{ &b }; 
    gsl::not_null<A*> pA{ &b };

    assert(pA == pB);
}
Updated 02/06/2017 05:00

Duplicate feed activity.

fetlife/android

Bug. Low priority. V2.6.22. Activity feed duplicates final story upon loading next page. {Hit rock bottom when scrolling. After app loads next set, scroll down. First story on next page is the same as the final story on the previous page.} *Randomly and unintentionally replicated issue 3 times in this version. **Multiple replications on previous versions.

Updated 06/07/2017 13:57 3 Comments

Compile without exception support

Microsoft/GSL

I am trying to use this in an other project which compiles without exception support.
I would not mind substituting the exceptions with asserts, and let it fire only in debug.

currently I use only the <gsl/span> header, my problem lies in <gsl/gsl_util>.
Could we get something like: ```

if defined ( __cpp_exceptions) || \

    (defined (_MSC_VER) && defined (__CPPUNWIND)) || \
    (defined (__GNUC__) && defined (__EXCEPTIONS))
#define ABORT_THROW (x) throw x

else

#define ABORT_THROW (x) (x, std::abort())

endif

`` and then useABORT_THROW` when needed?

Updated 10/07/2017 14:15 13 Comments

Consider reducing Boolean to Integral

ericniebler/stl2

From LWG Kona review of #155. Boolean is an enormously complicated hunk of specification for what is essentially intended to allow people to return integers from comparison operators in addition to bool. LWG made the suggestion to simply define Boolean as: c++ template <class T> concept bool Boolean() { return Integral<T>(); } which achieves the same goal with two lines of spec. At the very least, it’s a reasonable idea to apply this simplification for the TS and see if anyone complains.

Proposed Resolution

Replace the entire contents of [concepts.lib.compare.boolean] with:

template <class T>
concept bool Boolean() {
  return Integral<T>();
}
Updated 05/07/2017 17:57 20 Comments

Be consistent about explicitly requiring typename constraints

ericniebler/stl2

…in concept definitions. This came up in LWG Kona discussion of D0541R1’s definition of Assignable: c++ template <class T, class U> concept bool Assignable() { return is_lvalue_reference<T>::value && // see below CommonReference< const remove_reference_t<T>&, const remove_reference_t<U>&>() && requires(T t, U&& u) { { t = std::forward<U>(u) } -> Same<T>&&; }; } which forms const remove_reference_t<U>& without any assurance that type is valid (U could be void, for example). In many other places, e.g. Readable, we explicitly require the validity of types despite that substitution failure will handle an ill-formed type similarly. We need to consistently choose one or the other, or at least develop a guideline for when we use which approach.

Proposed Resolution

There are a great many issues in flight that alter the definitions of concepts in the TS, so it is not possible to list specific and detailed changes here without creating conflicts. We therefore propose application of the guideline developed later in this issue thread to the requires clauses - and particularly those that appear in concept definitions - in the TS. The application of that guideline can be summarized as: * Formation of types that can potentially fail shall occur only in a requires-expression; any requirements including a potentially failing type formation outside of a requires-expression should be transformed into nested requirements within a requires-expression. * the first potentially failing formation of a type shall be in a typename requirement for the type formed

This transformation should be sufficiently mechanical to be applied by the project editor without more detailed instruction. Note that the order of requirements in a requires clause is significant, so care should be taken not to reorder any requirements when applying this transformation.

Updated 16/07/2017 01:50 4 Comments

Add comments to the css files

systers/FirstAide-web

Pick only one file at a time. This is a beginner task. You can claim one css file and start working on it Make sure the format you use while adding comments is according to this https://perishablepress.com/obsessive-css-code-formatting-organization-comments-and-signatures/

Updated 24/07/2017 18:20 18 Comments

Consider removing `Writable`'s allowance that `operator*` may be a modification

ericniebler/stl2

Although this came up during review of the common_iterator wording from P0370 in Issaquah, the suggestion was that we consider making Writable’s * operator non-modifying in general. I vaguely recall that there was good reason for it to be modifying, if not exactly what that reason was.

IIRC this harkens back to the discussion about output iterator types that are their own proxy reference types, i.e., return a reference to *this from operator*. Should the assignment operator of such a proxy reference type be const - despite the intuition that assignment must modify something - or non-const, which requires operator* to return a mutable reference? I think I’ve come down on the wrong side of this question in the past and claimed that the assignment operator should be non-const, but the fact is that the assignment operator doesn’t modify the proxy reference itself; it modifies the referent.

This may be a case where we need to step outside the implicit rules and say that operator* is non-modifying despite that it is not required to be valid for constant Writables. Notably, every output iterator in N4606 - ostream_iterator, ostreambuf_iterator and the inserters - has a non-const operator*. Do we have any known examples of Writables that actually modify the Writable object itself in operator*?

Updated 16/07/2017 01:50 1 Comments

Usability: Settings screen has black text on a very dark background

fetlife/android

The “Settings” screen has black text on a very dark background which is very hard to read.

For a while I actually thought the “Settings” was empty (I’ve trying it out again from time to time, while going “Oh, still empty”) :) Also, I think the colors could give users the impression that the “Settings” screen is disabled.

All in all, the “Settings” screen should be consistent with the rest of the App, i.e. Dark/Black background and White text/font.

Updated 06/07/2017 14:04 13 Comments

rule suggestion T.12x Avoid recursion in variadic templates

isocpp/CppCoreGuidelines

Practical C++ Metaprogramming by @edouarda and @jfalcou highlighted a point that I think makes a good core guideline: avoid recursion in variadic templates.

This both dramatically improves compilation times and avoids recursive instantiation depth limits. The book talks about using pack expansions and std::index_sequence where possible, but perhaps there could be a note that even when those can’t be used, it is sometimes possible to rewrite recursion to log(n) depth.

Updated 24/07/2017 19:05 10 Comments

Kinky Emojis

fetlife/android

I am keep getting feedback via different sources why don’t we support kinky emojis in the App. I like the idea so merging those feedback I open a new issue here :)

As a side “help wanted” comment: Without any promise or commitment from our side… if anybody knows an already existing good kinky emoji let us know here!

Updated 06/07/2017 13:57 5 Comments

[M-GSL] Throwing copy and move constructors cause final_act to not execute the action

martinmoene/gsl-lite

On M-GSL, Eric Niebler:

throwing copy and move constructors cause final_act to not execute the action #283 .

The constructor of final_act here moves from argument f into member f_. If that throws, then the action is not executed, which is kinda the whole point of this utility. This paper shows how to do it right.


This paper: P0052R2 Example code on Peter Sommerlad’s GitHub

Updated 21/07/2017 15:24 1 Comments

F.40 : "Use a lambda.." : incorrect assertion that function objects don't overload?

isocpp/CppCoreGuidelines

The text says: “On the other hand, lambdas and function objects don’t overload; if you need to overload, prefer a function..”.

I was confused by this. The operator() for a “function object” is overloadable. e.g:

struct foo { void operator()(int) { std::cout << “first” << std::endl; } void operator()(double) { std::cout << “second” << std::endl; } };

Can the text be clarified?

Updated 16/06/2017 07:14 2 Comments

C.131 Avoid trivial getters and setters

isocpp/CppCoreGuidelines

One reason people write trivial getters and setters is to allow some future modifications without changing client code. The note in this item says, “A getter or a setter that converts from an internal type to an interface type is not trivial (it provides a form of information hiding).” A future change might be to change an internal type to a calculated type. Or to log every time the setter is called.

I also see this proposal http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n3888.pdf (Herb Sutter is an author), includes getters and setters such as: void set_tolerance(double tolerance); double get_tolerance(); I can’t see the implementation so I don’t know if that’s an internal type. But that’s the point - as a client of this code, I shouldn’t be exposed to whether it’s an internal type or not.

Updated 19/06/2017 18:49 19 Comments

Suggestion: ES.63: Distinguish between logical and bitwise comparison

isocpp/CppCoreGuidelines

Edit: Added text to Exceptions section, due to @BjarneStroustrup’s comment

ES.63: Distinguish between logical and bitwise comparison

Reason:

Say what you mean. If you want both the first and second term to be true, use operator&&. If you want to match bitflags, use operator&.

Matching bitflags can only be done sensibly using operator&, where doing a logical comparison can be done using one and two ampersands. In this case, use operator&&. There is no run-time speed to be gained using operator& instead.

The same story applies to operator| and operator||.

Examples:

Both terms being some value:

if (a == 42 && b == 3.14) { /* */ } //OK
if (a == 42 & b == 3.14) { /* */ } //Bad, will give compiler warning
if ((a == 42) & (b == 3.14)) { /* */ } //Bad, say what you mean

Bitflag:

if (i & flag_is_visible) { /* draw */ } //OK
if (i && flag_is_visible) { /* draw */ } //Runtime error

Alternatives: none

Exceptions:

If you don’t need the order dependence of && and you’ve measured that & is a significant performance optimization. If this is the case, it should be marked as an optimization. One way to do so is by making it an inline function, e,g, fast_and(0<=a,a<max).

Enforcement: compilers already give warnings

See alsos: none

Notes:

I saw the a == 42 & b == 3.14 in the wild, that’s why I suggested this rule, as the current guidelines do not state the preferred form explicitly yet.

Discussion:

Perhaps this item would better be renamed to a Non-Rule: NR x: operator& does not outperform operator&& in runtime performance

Updated 16/06/2017 11:18 6 Comments

NL.n Use namespaces, do nest your namespaces. Do not use a suffix or prefix in all defined names to provide scope.

isocpp/CppCoreGuidelines

Reason: Provides a narrow context where defined names are short, external names are well marked and also naturally segments your header-files, this makes larger code sets more easy to browse and comprehend and aids with refactoring.

Chances are your IDE provides a folding mechanism for namespaces, which is great when browsing these larger pieces of code.

Examples:

Alternatives:

Exceptions: extern “C”

Enforcement: Lint defined names and suggest shortening of long names, find repeated prefixes.

See also:

Notes: Do not use reversed Internet domain names as namespaces, as java frequently do for package names.

Discussion: Maybe GitHub could be persuaded to provide a global top-namespace allocatioin mechanism ?

Updated 19/06/2017 18:46 19 Comments

SEC-3190: RememberMe cookies cant handle username containing colon (":")

spring-projects/spring-security

Jeremy Waters (Migrated from SEC-3190) said:

I have confirmed this issue with TokenBasedRememberMeServices. The remember me cookie is a string of the form:

username + “:” + expiryTime + “:” + Md5Hex(username + “:” + expiryTime + “:” + password + “:” + key)

This is 3 tokens seperated by colons. sample:

doejohn:1767839462751:494b6435d6d0c2146bc608782543f805

When the username contains a colon, which is the default with spring-social, cookie decoding fails as it encounters 4 tokens (splitting the username into 2 separate tokens). sample:

twitter:1705891096:1767839462751:494b6435d6d0c2146bc608782543f805

It appears there is an existing hack to deal with urls containing colons (“https://…”) in AbstractRememberMeServices.decodeCookie(). I suggest urlencoding the value before creating the cookie string; and the url decoding the token when later retrieved from the cookie.

Updated 24/07/2017 16:19 1 Comments

N1: myth: a raw array is orders of magnitudes faster than std::vector

isocpp/CppCoreGuidelines

Edit: reworded ‘slow == Good’ to ‘slow due to’ thanks to @NicolBolas Edit: explicitly mention that the example is thanks to an initialization flaw, thanks to @scraimer, @cubbimew and @galik Edit: mention the discussion of @scraimer, @cubbimew and @galik in the Discussion section

N.1: myth: a raw array is orders of magnitudes faster than std::vector

Reason:

The C++ Core Guidelines suggest to use a std::vector of a raw array (see ‘SL.10: Prefer using STL array or vector instead of a C array’). There is a myth that for run-time speed, one should use arrays. A std::vector can never be faster than an array, as it has (a pointer to the first element of) an array as one of its data members. But the difference in run-time speed is slim and absent in any non-trivial program.

One reason for this myth to persist, are examples that compare raw arrays with mis-used std::vectors. For example, when constructing a big std::vector without reserve, arrays can indeed be orders of magnitude faster, due to copying of the std::vector its elements when exceeding its capacity. Prefer to learn the correct workings of std::vector over falling back to the use of raw arrays.

A second reason for this myth to persist is when comparing arrays and std::vector in debug mode. Where a std::vector has mechanisms in place that will help you during debugging, array are indifferent between debug and release mode. That std::vector is slower in debug mode is due to range checks, which will save time debugging run-time errors. When doing a benchmark, do this in release mode.

Examples:

This example shows how incorrect initialization of a std::vector helps this myth persist.

This example is slightly modified from the first hit when Googling ‘benchmark array versus std::vector’ (the post is here).

Here is the benchmark code shown to be twice as slow as when using a raw array:

std::vector<Pixel> pixels;
pixels.resize(dimension * dimension); //Create elements that need to be modified
for(int i = 0; < dimension * dimension; ++i)
{
    // Modify each element
    pixels[i].r = 255;
    pixels[i].g = 0;
    pixels[i].b = 0;
}

This code is indeed twice as slow, as there are twice as much Pixels allocated. The correct equivalent would be:

std::vector<Pixel> pixels(dimension * dimension, Pixel(255,0,0)); //Create desired elements directly 

When running the benchmark in release mode, the run-time speeds are equivalent.

Note that still the comparison is unfair (in the favor of an array), as is compares a static array versus a dynamic array (in the std::vector).

Alternatives:

If you do not need resizing, std::array may suit your needs and give better performance at debug mode.

Exceptions:

There may be cases in which you need good performance in debug mode as well.

Enforcement:

See alsos: none

Notes:

Discussion:

It may be that optimizers once were better in optimizing calls to operator[] on a plain array, instead of a std::vector. Nowadays there is little evidence that this still is the case (see for example std::vector versus array decayed into a pointer

Updated 19/06/2017 18:59 20 Comments

Suggestion: when using a library, follow its philosophy, instead of your own

isocpp/CppCoreGuidelines

Edit: Thanks @galik for correcting me! Edit: More focus on the word ‘framework’, thanks to @galik

?.1: when working within a framework of a library, follow its philosophy, instead of your own

Reason:

The C++ Core Guidelines give a set of general rules. Existing libraries may have different philosophies (and good reasons for this). When within a framework of a library, follow its philosophy, instead of bending it to follow your own (or the C++ Core Guidlines its) guidelines. Doing so will increase your productivity.

Examples:

_memory management_

The STL supplies the std::unique_ptr and std::shared_ptr for memory management. The Qt library facilitates a parent-child architecture, in which a parent deletes it children.

When writing your own Qt class (a class that inherits from QObject), stay within that Qt framework: instead of using STL smart pointers (for example, std::shared_ptr), use the Qt mechanism instead (for example, QSharedPointer).

_signal-slot mechanism_

The Boost libraries have a library (Boost.Signals2) to implement a signal-slot mechanism. The Qt library has a custom own signal-slot mechanism.

When writing your own Qt class (a class that inherits from QObject), stay within that Qt framework: instead of using Boost.Signals2 classes, use the Qt signal-slot machanism.

Alternatives:

Exceptions:

Enforcement:

See alsos:

Notes:

The libraries given in the examples are just examples.

Discussion:

Updated 16/06/2017 11:25 2 Comments

ES.2x redundancy

isocpp/CppCoreGuidelines

ES.20 takes up about 10% of the text of the whole ES section. It hits a lot of good points, but doesn’t provide (stable) links to sub-points for citation in code review or the like. (Yes, you can cite the examples or notes, but those are numbered globally and will change over time.)

Relatedly: ES.21 and ES.22 basically just restate ES.20.

I’d suggest adding stable links into sub-sections of ES.20 and dropping 21 and 22 entirely. Alternatively, the text of 20 can be spread out between 21 and 22, but I think it’d be hard to ever be completely sure when to cite one or the other.

If there’s editorial consensus, I’m happy to take a stab at whatever direction you like.

Updated 19/06/2017 18:56

SEC-3131: JdbcMutableAclService can be very slow to update ACLs with many children, grandchildren etc

spring-projects/spring-security

Simon van der Sluis (Migrated from SEC-3131) said:

The JdbcMutableAclService method public MutableAcl updateAcl(MutableAcl acl) calls clearCacheIncludingChildren(ObjectIdentity objectIdentity), which recursively works it’s way through all of an ObjectIdentities children and their children etc.

We have a very hierarchical structure controlled by ACLS, when this structure gets large, and we update the ACLs of the root object, this recursion is a killer, as each call to clearCacheIncludingChildren(.) involves a DB query.

I have worked around this problem by implementing my own JdbcMutableAclService which instead of recursively clearing the ObjectIdentities simply calls aclCache.clearCache(). (Hooray for dependency injection.)

Performance improved from ~3 seconds to sub second on a smallish data set.

I’m not sure if the hammer approach to clearing the cache is suitable for all situations, but the current recursive approach isn’t either.

Perhaps JdbcMutableAclService should support different strategies for controlling the cache.

Updated 01/06/2017 21:14 2 Comments

finally() mishandles modifiable lvalues

Microsoft/GSL

Meng Zhu pointed this out to me. gsl.h contains:

template <class F>
class final_act
{
[...]
private:
    F f_;
    bool invoke_;
};

// finally() - convenience function to generate a final_act
template <class F>
final_act<F> finally(const F &f) noexcept { return final_act<F>(f); }

template <class F>
final_act<F> finally(F &&f) noexcept { return final_act<F>(std::forward<F>(f)); }

Given a const lvalue of type X, finally(const F&) is selected, deducing F to be X, and it returns final_act<X>.

However, given a modifiable lvalue of type X, finally(F&&) is selected, deducing F to be X&, and it returns final_act<X&>. This appears to be completely undesired.

If final_act<F> assumes that F is an object type, it should static_assert so, and finally() should be fixed accordingly. The most robust fix would be to provide a perfect forwarder only, and use decay.

Lastly, all other occurrences of perfect forwarding in the GSL should be audited for this problem, especially when perfect forwarders are overloaded with anything else of the same arity (perfect forwarders are extremely greedy, and will outcompete other overloads, often unintentionally).

Updated 17/08/2017 18:21 3 Comments

SEC-3072: Provide Freemarker macro library

spring-projects/spring-security

Angel D. Segarra (Migrated from SEC-3072) said:

Spring Framework Web MVC currently ships Freemarker and Velocity macro libraries along with the JSP taglib , but Spring security ships only a JSP taglib which leaves users of the other technologies without good options out of the box. To make matters worse JspTaglibs hash in Freemarker no longer works in Spring Boot. I am requesting support parity with at least Freemarker to match Spring Web MVC.

Updated 06/07/2017 13:43 4 Comments

SES-166: Consider using OpenSAML 2.6.4 (or above)?

spring-projects/spring-security-saml

Thomas Maslen (Migrated from SES-166) said:

If I understand correctly, spring-security-saml2-core (both in 1.0.1.RELEASE and in master) is using OpenSAML 2.6.1 (as 1.0.0.RELEASE did).

That’s not terrible, but there are a couple of fine reasons for moving to OpenSAML 2.6.4 or above (IIRC latest is 2.6.5): - It fixed an XML vulnerability - In the course of doing that it got rid of all the awkward stuff that wanted to have endorsed JARs for some of the XML libraries, so it’s a lot easier now to have e.g. a nice, self-contained WAR file

[OpenSAML 3 has also been released (3.0.0, 3.1.0 and 3.1.1) and OpenSAML 2 may be headed toward legacy status, but the upgrade to 2.6.4+ is easy whereas moving to 3.* may be nontrivial].

[By the way, JIRA lists saml-1.0.0 and saml-1.0.1 under “Unreleased versions”]

Updated 03/07/2017 09:37 37 Comments

SEC-2914: Support @AuthenticationPrincipal on @SubscribeMapping endpoints

spring-projects/spring-security

Igor Kolomiets (Migrated from SEC-2914) said:

I’m using latest websocket/messaging related features of Spring Security 4 to secure my application’s websocket messaging endpoints. We have many @SubscribeMapping annotated methods to provide request-response style of communication that need access to current authenticated user. Currently we do this to get authenticated user:

@SubscribeMapping(“/foo”) public MyResponse foo(MyAuthenticationToken authenticationToken) { MyUser user = authenticationToken.getPrincipal(); }

Naturally I’d like to use @AuthenticationPrincipal so we can do this:

@SubscribeMapping(“/foo”) public MyResponse foo(@AuthenticationPrincipal MyUser user) {

}

But it looks like @AuthenticationPrincipal is only supported by @MessageMapping annotated methods. Can we have it work for those annotated with @SubscribeMapping?

Updated 20/06/2017 10:01 1 Comments

SEC-2856: Make cookie theft detection in remember-me service configurable because it's seriously broken

spring-projects/spring-security

Jean-Pierre Bergamin (Migrated from SEC-2856) said:

After enabling remember-me authentication for our SSO portal, people were complaining about errors they got while logging in. Those errors turned out to be CookieTheftExceptions.

After investigating quite intensively how these exceptions occured, we found that there are so many regular usecases how this can happen that this feature can be considered as really broken.

h5. Usecase 1 - Open two windows in your browser and login to the remember-me enabled web app in both windows - Close the browser - Open the browser (with the setting to re-open all previous windows) - Both windows get re-opened and both send almost simultaneously a request with the same remember-me cookie to the web app - The first request succeeds, where the second one fails (because the first already consumes the cookie) and the user is logged out

h5. Usecase 2 - Log in to the remember-me enabled web-app - Close the browser - Open the browser and visit the web-app again, which triggers a remember-me authentication - The remember-me authentication takes a while (e.g. because the AD-Server responds very slowly) and the user closes the tab - The user visits the web-app again after a while and gets a CookieTheftException and is logged out

The problem here is that the browser never got the response with the updated cookie back because the user closed the tab before.

h5. Usecase 3 - Open your remember-me enabled web-app in Chrome - Close the browser - Start entering the URL of your web-app in Chrome’s address bar and hit enter - You get a CookieTheftException and are logged out

What happens here is that Chrome already sends a request in the background while entering the URL. When hitting enter before the background request returned with a new cookie in its response, a second request with the same cookie is sent again - which leads to a CookieTheftException.

h5. Usecase 4 - The remember-me enabled web-app is an SSO (single sign-on) application where people authenticate for different other web-apps - Open different web-apps which use the SSO in different tabs - Close the browser - Open the browser again (with the setting to re-load all previous tabs) - The different web-apps in the different tabs need to re-login with the SSO app and immediately redirect to it after loading - You get a CookieTheftException and are logged out

The problem here is that all webapps redirect to the SSO app and query it almost simultaneously which leads to the CookieTheftException.

As you can see, this CookieTheftException detection makes more harm than it tries to resolve. The PersistentTokenBasedRememberMeServices should have a way to disable the cookie theft detection on demand.

Currently we “disable” the cookie theft detection by always returning a constant token data like:

public class CustomPersistentTokenBasedRememberMeServices extends PersistentTokenBasedRememberMeServices {
    public CustomPersistentTokenBasedRememberMeServices(String key, UserDetailsService userDetailsService, PersistentTokenRepository tokenRepository) {
        super(key, userDetailsService, tokenRepository);
    }

    @Override
    protected String generateTokenData() {
        // Return a constant value for the token value to avoid CookieTheftExceptions.
        return "U1WUsKXNkM0Jzpozau/BeQ==";
    }
}

The PersistentTokenBasedRememberMeServices class should be configurable to have cookie theft detection turned on or off.

Updated 17/08/2017 14:05 4 Comments

SEC-2721: SaveContextOnUpdateOrErrorResponseWrapper.SaveContextServletOutputStream does not implement Servlet 3.1 abstract methods

spring-projects/spring-security

Jon Nermut (Migrated from SEC-2721) said:

If one tries to call setWriteListener on a ServletOutputStream on a stream that has been wrapped by SaveContextOnUpdateOrErrorResponseWrapper.SaveContextServletOutputStream, you get an abstract method exception, as SaveContextServletOutputStream does not implement and delegate these new methods in the Servlet 3.1 spec:

    /**
     * Checks if a non-blocking write will succeed. If this returns
     * <code>false</code>, it will cause a callback to
     * {@link WriteListener#onWritePossible()} when the buffer has emptied. If
     * this method returns <code>false</code> no further data must be written
     * until the contain calls {@link WriteListener#onWritePossible()}.
     *
     * @return <code>true</code> if data can be written, else <code>false</code>
     *
     * @since Servlet 3.1
     */
    public abstract boolean isReady();

    /**
     * Sets the {@link WriteListener} for this {@link ServletOutputStream} and
     * thereby switches to non-blocking IO. It is only valid to switch to
     * non-blocking IO within async processing or HTTP upgrade processing.
     *
     * @param listener  The non-blocking IO write listener
     *
     * @throws IllegalStateException    If this method is called if neither
     *                                  async nor HTTP upgrade is in progress or
     *                                  if the {@link WriteListener} has already
     *                                  been set
     * @throws NullPointerException     If listener is null
     *
     * @since Servlet 3.1
     */
    public abstract void setWriteListener(javax.servlet.WriteListener listener);

It just needs to delegate them through to the wrapped object.

Updated 05/06/2017 23:32 4 Comments

SEC-2712: Allow WithSecurityContextTestExecutionListener to execute after @Before

spring-projects/spring-security

Rob Winch (Migrated from SEC-2712) said:

Hi Rob, great enhancement. Would it be possible somehow to to invoke @WithUserDetails after @Before annotated method or have the execution order configurable? I think it’d be great to be able to create a new fresh user account in some sort of @Before method and then authenticate it with @WithUserDetails. I’m trying to avoid creating new user in @BeforeClass because each @test method can alter user’s information, so I configured test to rollback transaction after each @test and create a new user before, however @WithUserDetails tries to call UserDetailsService.loadUserByUsername() before actual user was created in @Before. Any ideas? Thanks a lot!

Updated 24/07/2017 16:19 5 Comments

SEC-2427: Subsequent requests from the same browser break remember me function and throws CookieTheftException

spring-projects/spring-security

Vertonur Sunimi (Migrated from SEC-2427) said:

Prerequisite: Browser with authenticated rememberme cookie stored.

Reproduction steps: 1. The browser open a page to trigger auto login. 2. Request received by server and processed right before code tokenRepository.updateToken(newToken.getSeries(), newToken.getTokenValue(), newToken.getDate()); of PersistentTokenBasedRememberMeServices and the executing thread paused. 3. End user refresh the page and a second request is sent to the server 4. The second request is recieved and processed through the Spring Security filters and returned a new cookie to the browser and the token( token-A) in the db is updated either. 5. The first request resumed and run code updateToken thus the db is updated with the new generated token (token-B). As the request has been canceled by the browser so token-B will never reach the browser with code addCookie(newToken, request, response); 6. Session of the end user time out and pages are requested again, browser send request s with token-A 7. !presentedToken.equals(token.getTokenValue()) of PersistentTokenBasedRememberMeServices is checked thus caused CookieTheftException be thrown and all tokens related to the end user in db are deleted.

SO concurrency control is needed for rememberme filter.

Updated 17/08/2017 12:50 2 Comments

SEC-2409: Spring Security / Spring Data Acl Integration

spring-projects/spring-security

Rob Winch (Migrated from SEC-2409) said:

Spring Security’s ACL implementation allows users to determine if a access is allowed after the results come back from the database. This works when there is a small number of results, but breaks down when paging is necessary. Now that we have Spring Data, it would be good to provide integration with Spring Data to ensure that the query’s are automatically updated based upon the security restrictions. We could provide a default strategy that aligns with Spring Security’s ACL model.

Updated 10/06/2017 17:47 26 Comments

SEC-2224: ActiveDirectoryLdapAuthenticationProvider throws BadCredentialsException if userPrincipalName not equal to sAMAccountName + @domain

spring-projects/spring-security

Michael Solano (Migrated from SEC-2224) said:

When using the sAMAccountName for authentication via ActiveDirectoryLdapAuthenticationProvider, a BadCredentialsException will be thrown if the userPrincipalName is not the sAMAccountName with @domain post-fixed.

For example, if the sAMAccountName is “bwayne” but the userPrincipalName is “bruce.wayne@batcave.net”, authentication will fail. The createBindPrincipal method assumes the userPrincipalName will be “bwayne@batcave.net” and not “bruce.wayne@batcave.net”.

The code below shows the details of that method:

    String createBindPrincipal(String username) {
        if (domain == null || username.toLowerCase().endsWith(domain)) {
            return username;
        }

        return username + "@" + domain;
    }
Updated 18/07/2017 18:52 11 Comments

SEC-1945: Encapsulation of Pre/PostAuthorize expressions in custom annotations and combining them on methods

spring-projects/spring-security

Jan Novotný (Migrated from SEC-1945) said:

I would like to raise an idea for further discussion. When using PreAuthorize / PostAuthorize annotations on methods in a larger scale there is a lot of redundancy and repetition of some expression parts. This makes code less readable and might introduce bugs just because developer looses perspective on the code. I described my idea in more detail in article on my blog and would be happy if it would inspire you somehow to extend current features of Spring Security. If you wouldn’t consider it to be a good idea, please close this issue immediately.

http://blog.novoj.net/2012/03/27/combining-custom-annotations-for-securing-methods-with-spring-security/

Thank you for your time.

Might be connected in certain aspects with these issues: https://jira.springsource.org/browse/SEC-1629 https://jira.springsource.org/browse/SEC-1796

Updated 03/07/2017 08:15 5 Comments

SEC-972: Allow use of non-numeric (i.e. string) values for ObjectIdentity.getIdentifier() with the default JDBC implementation

spring-projects/spring-security

“Zachary Roadhouse”:https://jira.spring.io/secure/ViewProfile.jspa?name=zroadhouse said:

I am integrating the Spring Security ACL module into our application which uses a mixture of UUID and numeric primary keys as our domain object identifiers. Based on reading the interface documentation fo ObjectIdentity, I had hoped that our UUID identifiers would work with the JdbcMutableAclService implementation without any customizations to the source. This is not currently true as the JdbcMutableAclService and related classes all assume that the identifier value can be converted to a Long. This assumption is limited to a small number of points in the implementation.

This is a request to change the implementation to allow the choice of Long vs. String to be made into a configuration setting (or alternate implementation).

Updated 28/06/2017 21:23 17 Comments

Fork me on GitHub