[1.0.x] AI File to Text

Thank you for applying!

Before giving links helpful to understand how the review process works, what to expect from a review, and what to do to avoid a review takes more time than needed, I would like to thank all the reviewers for the work they do.
These applications are volunters-driven, which also means it is not possible to predict when an application will be marked fixed and the applicant will get the permission to opt projects into security advisory policy. While we aim to make an application as quick as possible, it is also important for us that more people review the project used for an application. In this way, we make sure applications do not miss some important points that should be instead reported.
Applications are not meant to be complete debugging sessions that eliminate every existing bug, though. I apologize if sometimes applications seem to go into too-detailed reviews.

Please read Review process for security advisory coverage: What to expect for more details and Security advisory coverage application checklist to understand what reviewers look for. Tips for ensuring a smooth review gives some hints for a smoother review.

The important notes are the following.

• If you have not done it yet, you should enable GitLab CI for the project and fix the PHP_CodeSniffer errors/warnings it reports.
• For the time this application is open, only your commits are allowed.
• The purpose of this application is giving you a new drupal.org role that allows you to opt projects into security advisory coverage, either projects you already created, or projects you will create. The project status will not be changed by this application; once this application is closed, you will be able to change the project status from Not covered to Opt into security advisory coverage. This is possible only 14 days after the project is created.
  
  Keep in mind that once the project is opted into security advisory coverage, only Security Team members may change coverage.
• Only the person who created the application will get the permission to opt projects into security advisory coverage. No other person will get the same permission from the same application; that applies also to co-maintainers/maintainers of the project used for the application.
• We only accept an application per user. If you change your mind about the project to use for this application, or it is necessary to use a different project for the application, please update the issue summary with the link to the correct project and the issue title with the project name and the branch to review.

To the reviewers

Please read How to review security advisory coverage applications, Application workflow, What to cover in an application review, and Tools to use for reviews.

The important notes are the following.

• It is preferable to wait for a project moderator before posting the first comment on newly created applications. Project moderators will do some preliminary checks that are necessary before any change on the project files is suggested.
• Reviewers should show the output of a CLI tool only once per application.
• It may be best to have the applicant fix things before further review.

For new reviewers, I would also suggest to first read In which way the issue queue for coverage applications is different from other project queues.

1. FILE: README.md

The README file is missing the required section - Configuration.

2. FILE: ai_file_to_text.module

For a new module that aims to be compatible with Drupal 10 and Drupal 11, I would rather implement hooks as class methods as described in Support for object oriented hook implementations using autowired services.
It would require increasing the minimum Drupal 10 version supported, but Drupal 10.1 is no longer supported.

3. FILE: src/Pdf/HtmlRenderer.php

  /**
   * The style analyzer for heading detection and inline styles.
   *
   * @var \Drupal\ai_file_to_text\Pdf\StyleAnalyzer
   */
  protected StyleAnalyzer $styleAnalyzer;

  /**
   * Constructs an HtmlRenderer.
   *
   * @param \Drupal\ai_file_to_text\Pdf\StyleAnalyzer $style_analyzer
   *   The style analyzer instance.
   */
  public function __construct(StyleAnalyzer $style_analyzer) {
    $this->styleAnalyzer = $style_analyzer;
  }

FILE: src/Pdf/LinkExtractor.php

  /**
   * The style analyzer for color conversion.
   *
   * @var \Drupal\ai_file_to_text\Pdf\StyleAnalyzer
   */
  protected StyleAnalyzer $styleAnalyzer;

  /**
   * Constructs a LinkExtractor.
   *
   * @param \Drupal\ai_file_to_text\Pdf\StyleAnalyzer $style_analyzer
   *   The style analyzer instance.
   */
  public function __construct(StyleAnalyzer $style_analyzer) {
    $this->styleAnalyzer = $style_analyzer;
  }

New modules, which are compatible with Drupal 10 and higher versions are expected to include type declarations in property definitions, and use constructor property promotion.

I don’t see any changes in the 1.0.x branch.

FILE: ai_file_to_text.module

Since the module is declared compatible with Drupal 10.3, removing the function implementing the hook is not possible. The function still needs to be defined, but it calls the method defined by the service class, as described in Support for object oriented hook implementations using autowired services (Backwards-compatible Hook implementation for Drupal versions from 10.1 to 11.0).

Rest seems fine to me.

Please wait for other reviewers and Project Moderator to take a look and if everything goes fine, you will get the role.

The README is technically thorough, but deviates from the drupal.org README template in structure and scope.

Although not strictly required it is good practice to provide a readme that follows the the drupal.org README template

AI-oriented content — a significant portion of the README (Architecture, Key classes table, capability groups, ExtractorInterface method table) reads more like structured context for AI agents or LLMs than traditional developer documentation. At 349 lines, the README functions as an architecture reference document. Consider moving the deep technical content (lines 14–76: architecture, dynamic type registration, capability groups, key classes table) to a separate AGENTS.md or developer documentation, and keeping the README focused on getting started.

• The following points are just a start and don't necessarily encompass all of the changes that may be necessary
• A specific point may just be an example and may apply in other places
• A review is about code that does not follow the coding standards, contains possible security issue, or does not correctly use the Drupal API
• The single review points are not ordered, not even by importance

src/Pdf/HtmlRenderer.php

      if ($level > 0) {
        $native_headings = !empty($options['native_headings']);
        if ($native_headings) {
          $tag = 'h' . $level;
        }
        else {
          $tag = $level === 1 ? 'p' : ('h' . $level);
        }
        $style_attr = $styles !== '' ? ' style="' . $styles . '"' : '';
        $html .= '<' . $tag . ' class="h' . $level . '"' . $style_attr . '>' . $inner_html . '</' . $tag . '>' . "\n";
      }
      else {
        if ($styles !== '') {
          $html .= '<p style="' . $styles . '">' . $inner_html . '</p>' . "\n";
        }
        else {
          $html .= '<p>' . $inner_html . '</p>' . "\n";
        }
      }

When strings are concatenated to obtain HTML markup to output, it is probably better to use a theme function or a render element as described in Render API overview.

  protected function renderList(array $block, float $body_size, ?array $body_color): string {
    $tag = $block['listType'] === 'ol' ? 'ol' : 'ul';
    $html = '<' . $tag . '>' . "\n";

    $items = $block['items'];
    $i = 0;
    while ($i < count($items)) {
      $item = $items[$i];
      $text = $item['listText'] ?? $item['text'];
      // Apply annotation-based link spans before escaping.
      $text = $this->applyLinkSpans($text, $item['linkSpans'] ?? []);
      $escaped = htmlspecialchars($text, ENT_QUOTES, 'UTF-8');
      $escaped = $this->autoLinkUrls($escaped);
      $escaped = $this->restoreAnchorTags($escaped);

      // Check if next items are sub-items.
      $sub_items = [];
      while ($i + 1 < count($items) && !empty($items[$i + 1]['isSubItem'])) {
        $i++;
        $sub_items[] = $items[$i];
      }

      if (!empty($sub_items)) {
        $html .= '<li>' . $escaped . "\n";
        $html .= '<ul>' . "\n";
        foreach ($sub_items as $sub) {
          $sub_text = $sub['listText'] ?? $sub['text'];
          $sub_text = $this->applyLinkSpans($sub_text, $sub['linkSpans'] ?? []);
          $sub_escaped = htmlspecialchars($sub_text, ENT_QUOTES, 'UTF-8');
          $sub_escaped = $this->autoLinkUrls($sub_escaped);
          $sub_escaped = $this->restoreAnchorTags($sub_escaped);
          $html .= '<li>' . $sub_escaped . '</li>' . "\n";
        }
        $html .= '</ul>' . "\n";
        $html .= '</li>' . "\n";
      }
      else {
        $html .= '<li>' . $escaped . '</li>' . "\n";
      }

      $i++;
    }

    $html .= '</' . $tag . '>' . "\n";
    return $html;
  }

Drupal core has a theme function to render ordered and unordered lists.

  protected function renderTable(array $rows, float $body_size, ?array $body_color): string {
    if (empty($rows)) {
      return '';
    }

    $html = "<table>\n";

    foreach ($rows as $ri => $row) {
      $cells = $row['cells'] ?? [];
      $cell_tag = ($ri === 0) ? 'th' : 'td';

      if ($ri === 0) {
        $html .= "<thead>\n";
      }
      elseif ($ri === 1) {
        $html .= "<tbody>\n";
      }

      $html .= '<tr>';
      foreach ($cells as $cell_text) {
        $escaped = htmlspecialchars(trim($cell_text), ENT_QUOTES, 'UTF-8');
        $html .= '<' . $cell_tag . '>' . $escaped . '</' . $cell_tag . '>';
      }
      $html .= "</tr>\n";

      if ($ri === 0) {
        $html .= "</thead>\n";
      }
    }

    if (count($rows) > 1) {
      $html .= "</tbody>\n";
    }

    $html .= "</table>\n";

    return $html;
  }

Drupal core has also a theme function to render tables.
Drupal modules are supposed to return render array, not to build HMTL markup by concatenating strings. The task of returning a string from a render array is the task of render elements and theme functions.

src/Pdf/PdfToHtml.php

Code compatible with latest Drupal releases should use property promotion.

The files contained in the PDF directory does not seem to contain Drupal-related code, nor is that code for plugins. Each class should at least define a service class and used as service.

I am changing priority as per Issue priorities.

I am changing priority as per Issue priorities.

Reviewed the 1.0.x branch, focusing on the document-parsing surface. Several things are handled well: the Poppler calls pass file paths through escapeshellarg() (no command injection), the AiFunctionCall entry point checks $file->access('view', $account) before reading, the HTML-building extractors escape content with htmlspecialchars(ENT_QUOTES), and Poppler temp files are unlink()ed.

Points worth addressing:

• No size/complexity guard before parsing untrusted documents. OdtExtractor reads content.xml from the uploaded zip and loadXML()s it, and WordExtractor/SpreadsheetExtractor call IOFactory::load() on the whole file, with no upstream limit. A small, highly compressed ODT/XLSX (a decompression bomb) or an oversized XML can exhaust memory during a request. Extraction is reachable by anyone who can supply a file (file field / AI automator / AI function call), so a max file-size — and for zips a max uncompressed-size — check before parsing would be worthwhile.
• Third-party dependency provenance. composer.json requires ahmaadkhader/pdf-to-html ^1.0, a low-profile single-maintainer Packagist package on which the PDF path depends. For a security-covered module it is worth confirming its maintenance and provenance (or vendoring/replacing it), since the module's PDF security posture inherits from it.
• XML entity hardening (defence in depth). OdtExtractor's DOMDocument::loadXML() and PopplerPdfExtractor's simplexml_load_string() use default flags. Because the module requires ^10.3 (PHP 8.1+), libxml does not resolve external entities by default and no LIBXML_NOENT/DTDLOAD is passed, so XXE is not exploitable as written — but passing LIBXML_NONET explicitly is cheap and future-proofs against a PHP/config change. Also worth confirming the required PhpWord/PhpSpreadsheet versions are the XXE-hardened releases.

I use Assisted AI but I review what it generates

Follow-up review of a9750af on 1.0.x. The decompression-bomb guard is implemented the right way: it reads the archive's central directory via ZipArchive::statIndex (which decompresses nothing), refuses files whose entry count or total declared uncompressed size exceed the limits, logs the refusal, and the limits are overridable per site in settings.php. It sits ahead of the single extract() call that both the document-loader and AI function-call paths flow through, and the new unit tests cover the refusal paths.

The XML hardening matches what was suggested: LIBXML_NONET on both OdtExtractor loads and on the Poppler XML parse, with the code comments correctly noting that external entity resolution is already off by default on PHP 8.1+ and that this is defence in depth.

On the pdf-to-html dependency, the applicant owning the Packagist package answers the provenance question - the supply chain is the same author as the module itself.

Everything raised in my earlier review is addressed - ready for coverage from my side.

I use Assisted AI but I review what it generates

Comment updated above

Thank you for your contribution and for your patience with the review process!

I am going to update your account so you can opt into security advisory coverage any project you create, including the projects you already created.

These are some recommended readings to help you with maintainership:

• Dries' post on Responsible maintainers
• Maintainership
• Git version control system
• Issue procedures and etiquette
• Maintaining and responding to issues for a project
• Release naming conventions.

You can find more contributors chatting on Slack or IRC in #drupal-contribute. So, come hang out and stay involved!
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

I thank also all the reviewers for helping with these applications.