Skip to content

Commit 19b659d

Browse files
committed
pg: machine-enforce that org.bouncycastle.openpgp.operator does not import org.bouncycastle.openpgp.api, the dependency running api to operator and not back.
1 parent b70137e commit 19b659d

3 files changed

Lines changed: 34 additions & 1 deletion

File tree

‎config/checkstyle/checkstyle.xml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@
4343
<property name="path" value=".*[\\/]core[\\/]src[\\/]main[\\/]java[\\/].*"/>
4444
</module>
4545

46+
<module name="ImportControl">
47+
<property name="file" value="${config_loc}/import-control-pg-operator.xml"/>
48+
<property name="path" value=".*[\\/]pg[\\/]src[\\/]main[\\/]java[\\/]org[\\/]bouncycastle[\\/]openpgp[\\/]operator[\\/].*"/>
49+
</module>
50+
4651
<module name="org.bouncycastle.checkstyle.DebugMethodChecker">
4752
<property name="contains" value="(org\.bouncycastle\.util\.encoders\.)*Hex\.toHexString$"/>
4853
<property name="ignorePackages" value="(org\.bouncycastle\..*\.examples\.).*|(org\.bouncycastle\.util\.encoders\.Hex)"/>
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?xml version="1.0"?>
2+
<!DOCTYPE import-control PUBLIC
3+
"-//Checkstyle//DTD ImportControl Configuration 1.4//EN"
4+
"https://checkstyle.org/dtds/import_control_1_4.dtd">
5+
6+
<!--
7+
org.bouncycastle.openpgp.operator is the operator layer the high-level API is built on, so it
8+
must not depend on org.bouncycastle.openpgp.api (or any of its subpackages) - the dependency
9+
runs api -> operator and not back. A class that needs both belongs in openpgp.api.operator,
10+
where PGPContentSignerBuilderProviderFactory sits. Scoped to
11+
pg/src/main/java/org/bouncycastle/openpgp/operator by the ImportControl module's path property
12+
in checkstyle.xml; the subpackages (bc, jcajce, ...) are covered with it.
13+
-->
14+
<import-control pkg="org.bouncycastle.openpgp.operator" strategyOnMismatch="allowed">
15+
16+
<disallow pkg="org.bouncycastle.openpgp.api"/>
17+
18+
</import-control>

‎docs/claude/architecture.md‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,17 @@ Practical implications when adding code:
243243
- Need to wrap an existing `Jca*` builder? Either (a) wrap the JCA-free parent (e.g. wrap `SignerInfoGeneratorBuilder` instead of `JcaSignerInfoGeneratorBuilder`) so the class can stay at the top, or (b) move the class into the `.jcajce` subpackage.
244244
- A top-level class that does need to expose a JCA-friendly or lightweight-friendly factory method should ship the factory in its `.jcajce` or `.bc` peer instead of pulling JCA/lightweight imports into the top package.
245245

246-
The rule applies uniformly to `pkix` (`cms`, `cades`, `tsp`, `cert`, `operator`, ...), `pg`, `mail`/`jmail`, `tls`, and `mls`. When adding a new package under any of these modules, decide on the split up-front: if any class needs `java.security` / `javax.crypto` beyond `SecureRandom`, the package should be a `.jcajce` subpackage; if any class needs `org.bouncycastle.crypto.*`, the package should be a `.bc` subpackage. A JCA-free, lightweight-free top-level parent is usually still appropriate to host the operator interfaces both flavours adapt to.
246+
The rule applies uniformly to `pkix` (`cms`, `cades`, `tsp`, `cert`, `operator`, ...), `pg`, `mail`/`jmail`, `tls`, and `mls`.
247+
248+
`pg` carries one extra layering rule, and it is machine-enforced: **`org.bouncycastle.openpgp.operator`
249+
must not import `org.bouncycastle.openpgp.api`** (or any of its subpackages). The dependency runs
250+
api -> operator, not back; a class that needs both belongs in `org.bouncycastle.openpgp.api.operator`,
251+
where `PGPContentSignerBuilderProviderFactory` sits. A second `ImportControl` module in
252+
`config/checkstyle/checkstyle.xml` applies `config/checkstyle/import-control-pg-operator.xml` to
253+
`pg/src/main/java/org/bouncycastle/openpgp/operator` (subpackages included), so a new import fails
254+
`:pg:checkstyleMain` with `Disallowed import - org.bouncycastle.openpgp.api.<X>. [ImportControl]`.
255+
As with the core import control, checkstyle only sees imports - a fully-qualified
256+
`org.bouncycastle.openpgp.api.Foo` in a method body would slip past, so keep an eye out in review. When adding a new package under any of these modules, decide on the split up-front: if any class needs `java.security` / `javax.crypto` beyond `SecureRandom`, the package should be a `.jcajce` subpackage; if any class needs `org.bouncycastle.crypto.*`, the package should be a `.bc` subpackage. A JCA-free, lightweight-free top-level parent is usually still appropriate to host the operator interfaces both flavours adapt to.
247257

248258
## Shared prov/pkix cert-path logic goes in `asn1.x509` as a public pure-ASN.1 validator
249259

0 commit comments

Comments
 (0)