Skip to content
Snippets Groups Projects

#3295666

Open #3295666
issue/sessionless-3295666:1.x into 1.x
12 unresolved threads
Open Nitin Lama requested to merge issue/sessionless-3295666:1.x into 1.x
12 unresolved threads

#3295666 Addressing 10.1, 10.2, 10.3, 10.4, 10.5, 10.7, 10.8, 10.9

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
sessionless_forms/src/SessionlessFormCache.php
53 /**
54 * The logger channel.
55 *
56 * @var \Drupal\Core\Logger\LoggerChannelInterface
57 */
33 58 protected LoggerChannelInterface $logger;
34 59
60 /**
61 * The cryptography service.
62 *
63 * @var \Drupal\sessionless\CryptoService
64 */
35 65 protected CryptoService $cryptoService;
36 66
67 /**
68 * Constructs a new SessionlessFormCache object.
  • Alberto Paderno
    Alberto Paderno @avpaderno started a thread on an old version of the diff
    • sessionless_forms/src/SessionlessFormCache.php
    64 */
    35 65 protected CryptoService $cryptoService;
    36 66
    67 /**
    68 * Constructs a new SessionlessFormCache object.
    69 *
    70 * @param string $root
    71 * The app root.
    72 * @param \Drupal\Core\Extension\ModuleHandlerInterface $moduleHandler
    73 * The module handler.
    74 * @param \Drupal\Core\Session\AccountInterface $currentUser
    75 * The current user.
    76 * @param \Drupal\Core\Access\CsrfTokenGenerator $csrfToken
    77 * The CSRF token generator.
    78 * @param \Drupal\Core\Logger\LoggerChannelInterface $logger
    79 * The logger service.
  • Alberto Paderno
    Alberto Paderno @avpaderno started a thread on an old version of the diff
    • sessionless_forms/src/SessionlessFormCache.php
    32 /**
    33 * The module handler.
    34 *
    35 * @var \Drupal\Core\Extension\ModuleHandlerInterface
    36 */
    27 37 protected ModuleHandlerInterface $moduleHandler;
    28 38
    39 /**
    40 * The currently logged-in user account.
    41 *
    42 * @var \Drupal\Core\Session\AccountInterface
    43 */
    29 44 protected AccountInterface $currentUser;
    30 45
    46 /**
    47 * The CSRF token generator service.
  • Alberto Paderno
    Alberto Paderno @avpaderno started a thread on the diff
    • sessionless_forms/src/SessionlessFormsServiceProvider.php
    6 6 use Drupal\Core\DependencyInjection\ServiceProviderBase;
    7 7 use Symfony\Component\DependencyInjection\Reference;
    8 8
    9 /**
    10 * Service provider class.
    11 */
    • Comment on lines +9 to +11
      Alberto Paderno
      Alberto Paderno @avpaderno

      A definite article is missing. I am not sure that description is sufficient, as it applies for every service provider. The description should describe the class purpose.

    • Please register or sign in to reply
  • Alberto Paderno
    Alberto Paderno @avpaderno started a thread on the diff
    • sessionless_session/src/SessionlessSession.php
    15 * The sticky query storage implementation used for storing sticky queries.
    16 *
    17 * @var \Drupal\sticky_query\StickyQueryStorage\StickyQueryStorageInterface
    18 */
    14 19 protected StickyQueryStorageInterface $storage;
    15 20
    21 /**
    22 * Whether or not to encrypt stored data.
    23 *
    24 * @var bool
    25 */
    16 26 protected bool $encrypt;
    17 27
    28 /**
    29 * {@inheritdoc}
    30 */
  • Alberto Paderno
    Alberto Paderno @avpaderno started a thread on the diff
    • sessionless_session/src/SessionlessStickyQueryFactory.php
    8 8 use Drupal\sticky_query\StickyQuery\StickyQueryHandlerRegistry;
    9 9 use Drupal\sticky_query\StickyQueryStorage\StickyQuerySimpleStorage;
    10 10
    11 /**
    12 * Class to provide functionality for SessionlessStickyQueryFactory.
    13 */
  • Alberto Paderno
    Alberto Paderno @avpaderno started a thread on the diff
    • sessionless_session/src/SessionlessStickyQueryFactory.php
    17 * The cryptography service.
    18 *
    19 * @var \Drupal\Component\Utility\CryptoService
    20 */
    13 21 protected CryptoService $cryptoService;
    14 22
    23 /**
    24 * The sticky query simple storage service used for storing sticky queries.
    25 *
    26 * @var \Drupal\sticky_query\StickyQueryStorage\StickyQuerySimpleStorage
    27 */
    15 28 protected StickyQuerySimpleStorage $storage;
    16 29
    30 /**
    31 * {@inheritdoc}
    32 */
  • Alberto Paderno
    Alberto Paderno @avpaderno started a thread on the diff
    • sessionless_session/src/StickyQueryStorageEncryptionDecorator.php
    10 10 */
    11 11 final class StickyQueryStorageEncryptionDecorator implements StickyQueryStorageInterface {
    12 12
    13 /**
    14 * The underlying sticky query storage implementation being decorated.
    15 *
    16 * @var \Drupal\sticky_query\StickyQueryStorage\StickyQueryStorageInterface
    17 */
    13 18 protected StickyQueryStorageInterface $decorated;
    14 19
    20 /**
    21 * The cryptography service used for encryption and decryption.
    22 *
  • Alberto Paderno
    Alberto Paderno @avpaderno started a thread on the diff
    • sessionless_session/src/StickyQueryStorageEncryptionDecorator.php
    21 * The cryptography service used for encryption and decryption.
    22 *
    23 * @var \Drupal\sessionless\CryptoService
    24 */
    15 25 protected CryptoService $cryptoService;
    16 26
    27 /**
    28 * Whether or not encryption must be performed on stored data.
    29 *
    30 * @var bool
    31 */
    17 32 protected bool $mustEncrypt;
    18 33
    19 34 /**
    35 * Constructor for class.
    36 *
  • Alberto Paderno
    Alberto Paderno @avpaderno started a thread on the diff
    • src/CryptoKeyStorage.php
    19 19 */
    20 20 final class CryptoKeyStorage {
    21 21
    22 /**
    23 * {@inheritdoc}
    24 */
    22 25 protected StateInterface $state;
    23 26
    27 /**
    28 * {@inheritdoc}
    29 */
  • Alberto Paderno
    Alberto Paderno @avpaderno started a thread on an old version of the diff
    • src/CryptoService.php
    18 /**
    19 * The key storage object.
    20 *
    21 * @var \Drupal\Core\Encryption\CryptoKeyStorage
    22 */
    23 protected $keyStorage;
    24
    25 /**
    26 * The logger channel.
    27 *
    28 * @var \Drupal\Core\Logger\LoggerChannelInterface
    29 */
    30 protected $logger;
    31
    32 /**
    33 * Constructs a new CryptoService object.
  • Alberto Paderno
    Alberto Paderno @avpaderno started a thread on an old version of the diff
    • src/CryptoService.php
    104 148 return $base64;
    105 149 }
    106 150
    151 /**
    152 * {@inheritdoc}
    153 */
    107 154 protected function unpackData(string $base64) {
    108 155 $bytes = base64_decode($base64);
    109 156 if ($bytes === FALSE) {
    110 157 throw new \UnexpectedValueException('Not valid base64 data.');
    111 158 }
    112 159 // Unsrializing incoming data here is safe, as we only use signed data.
    113 $unserialized = @unserialize($bytes);
    114 if ($unserialized === FALSE && $bytes !== 'b:0;') {
    160 $unserialized = json_decode($bytes, TRUE);
    161 if ($unserialized === NULL && json_last_error() !== JSON_ERROR_NONE) {
  • Alberto Paderno
    Alberto Paderno @avpaderno

    I would also check all the documentation comments containing {@inheritdoc}. It feels like it has been used where it should not be.

  • Mustakim Farooqui added 1 commit

    added 1 commit

    • 1ee27560 - addressed all phpcs issues mentioned in comments.

    Compare with previous version

  • Alberto Paderno added 9 commits

    added 9 commits

    Compare with previous version

  • Alberto Paderno added 1 commit

    added 1 commit

    Compare with previous version

    • Please register or sign in to reply