コードを直接貼りつけて問題点を探す場合は文字数制限のためかクラッシュする感じがある。なので添付ファイルにして渡すと見ながら回答してくれる。
You:
AozoraEpub3.java:ファイル
問題点を探して
ChatGPT:
確認しました。AozoraEpub3.java 全729行を見たところ、明確なバグ候補と将来的に問題になりそうな箇所がいくつかあります。特に優先度が高いものから挙げます。
1. AutoMarginNombreSize の代入先が間違っている【重大】
ここは明らかにバグです。
Java
try { autoMarginPadding=Float.parseFloat(props.getProperty("AutoMarginPadding")); } catch (Exceptione) {}
try { autoMarginNombre=Integer.parseInt(props.getProperty("AutoMarginNombre")); } catch (Exceptione) {}
try { autoMarginPadding=Float.parseFloat(props.getProperty("AutoMarginNombreSize")); } catch (Exceptione) {}
最後の行で AutoMarginNombreSize を
Java
autoMarginPadding
へ代入しています。
しかし、直前に
Java
floatnobreSize=0.03f;
があり、さらに setImageParam() では
Java
..., autoMarginNombre, nobreSize
として渡しています。つまり、おそらく本来は、
Java
try { nobreSize=Float.parseFloat(props.getProperty("AutoMarginNombreSize")); } catch (Exceptione) {}
です。
AozoraEpub3
修正:
Java
try { nobreSize=Float.parseFloat(props.getProperty("AutoMarginNombreSize")); } catch (Exceptione) {}
これはかなり確実に直してよい箇所です。
2. encType をファイル間で使い回している【重大】
ここもバグになる可能性が高いです。
初期値:
Java
StringencType="AUTO";
そしてファイルごとの処理で、
Java
encauto=AozoraEpub3.getTextCharset(...);
if (Objects.equals(encauto, "SHIFT_JIS")) encauto="MS932";
if (encType.equals("AUTO")) encType=encauto;
となっています。
AozoraEpub3 +1
例えば、
book1.txt → UTF-8
book2.txt → MS932
を一度に指定すると、
book1 → UTF-8
book2 → UTF-8
となる可能性があります。
つまり AUTO が最初のファイルで検出した文字コードに固定されてしまいます。
本来は、
Java
StringencType=commandLine.hasOption("enc")
?commandLine.getOptionValue("enc")
: "AUTO";
として、ループ内で、
Java
StringcurrentEncType=encType;
if ("AUTO".equals(currentEncType)) {
currentEncType=getTextCharset(...);
if ("SHIFT_JIS".equals(currentEncType)) {
currentEncType="MS932";
}
}
のようにするのが安全です。
複数ファイル変換をする場合は特に要注意です。
3. jarPath の取得方法がWindows依存
Java
StringjarPath=System.getProperty("java.class.path");
intidx=jarPath.indexOf(";");
if (idx>0) jarPath=jarPath.substring(0, idx);
となっています。
AozoraEpub3
; はWindowsのclasspath separatorなので、Windows専用なら動きます。
ただし、
Java
System.getProperty("java.class.path")
は複数JARだけでなくディレクトリなども含み得るので、
java -cp ...
で起動した場合には jarPath="" になります。
その結果、
Java
newEpub3Writer("template/");
になります。
AozoraEpub3
JARとして起動する前提なら大問題ではありません。
ただ、CLIとして配布するなら、
Java
AozoraEpub3.class
の場所を取得する方法のほうが堅牢です。
4. coverFileName をファイル処理ループ内で変更している
ここは状態管理として危険です。
Java
if ("0".equals(coverFileName)) {
coverImageIndex=0;
coverFileName="";
} elseif ("1".equals(coverFileName)) {
coverFileName=AozoraEpub3.getSameCoverFileName(srcFile);
}
AozoraEpub3
コマンドラインで
-c 1
を指定すると、最初のファイルで
Java
coverFileName=getSameCoverFileName(srcFile);
に変わります。
すると2個目以降の入力ファイルでは「1」という指定情報が失われています。
同様に -c 0 でも、
Java
coverFileName="";
になってしまいます。
これは複数ファイル変換時の明確な問題候補です。
例えば、
Java
StringcoverOption=null;
を元のCLI指定値として保持し、
Java
StringcoverFileName=coverOption;
を各ファイルのループ内で作るほうが安全です。
5. AutoMargin の変数名が怪しい
Java
floatnobreSize=0.03f;
となっています。
AozoraEpub3
Nombre のことなら、
Java
nombreSize
などのほうが自然です。
ただし、これは機能バグではなく可読性の問題です。
さらに、
Java
autoMarginNombrenobreSize
という2つの名前が混在しているので、今回の AutoMarginNombreSize の代入ミスを誘発した可能性があります。
6. JpegQuality の読み込みがやや危険
Java
floatjpegQualty=0.8f;
try {
jpegQualty=Integer.parseInt(props.getProperty("JpegQuality")) /100f;
} catch (Exceptione) {}
AozoraEpub3
例えば設定値が
INI
JpegQuality=0.8
だと Integer.parseInt() で失敗して、黙って 0.8f に戻ります。
設定仕様が 80 のような整数なら問題ありません。
ただし、設定ファイル側で小数を許可するつもりなら、
Java
Float.parseFloat(...)
にする必要があります。
7. getTextCharset() のRAR処理でストリームを閉じていない
ここです。
Java
InputStreambis=newBufferedInputStream(newFileInputStream(tmpFile), 65536);
cs=Detector.getCharset(bis);
returncs;
AozoraEpub3
bis をcloseしていません。
Windowsではファイルハンドルが残って、特に大量のRARを処理した場合、
ファイルを削除できない
などにつながる可能性があります。
例えば、
Java
try (InputStreambis=newBufferedInputStream(newFileInputStream(tmpFile), 65536)) {
returnDetector.getCharset(bis);
}
にできます。
8. getTextInputStream() のZIP入力ストリームは正常だが、呼び出し側依存
ZIPの場合、
Java
ZipArchiveInputStreamzis= ...
...
returnzis;
となっています。
AozoraEpub3
これは「該当エントリまで移動した状態のZIPストリームを返す」という実装なので、設計としては成立します。
ただし、呼び出し側が必ずcloseする必要があります。
getBookInfo() では、
Java
is.close();
しています。
AozoraEpub3
convertFile()では、
Java
epubWriter.write(...)
側がcloseする前提です。
AozoraEpub3
コメントにもその前提がありますが、APIとしては少し壊れやすいです。
9. coverFileName のURL判定が弱い
Java
!coverFileName.startsWith("http")
AozoraEpub3
なので、
https://...
http://...
は通りますが、
HTTP://...
HTTPS://...
などはローカルファイルとして扱われます。
またURLとして許可するなら、
Java
URI
で判定したほうが堅牢です。
10. ファイル名の250文字制限が「文字数」であってWindowsのパス長を考慮していない
Java
if (outFileName.length() >250)
outFileName=outFileName.substring(0, 250);
AozoraEpub3
問題は、
C:\Users...\Documents...
まで含めて250文字にしていることです。
そのため出力ファイル名自体は短くても、環境によっては別の問題が出ます。
また、
Java
outFileName.substring(0, 250)
によってUnicodeサロゲートペアを途中で切る可能性もあります。
優先順位を付けると
| 優先度 |
箇所 |
問題 |
| 🔴 高 |
182–184 |
AutoMarginNombreSize → autoMarginPadding の代入ミス |
| 🔴 高 |
368–370 |
encType が複数入力ファイル間で固定される |
| 🔴 高 |
323–328 |
coverFileName をループ内で書き換える |
| 🟠 中 |
659–668 |
RARの InputStream close漏れ |
| 🟠 中 |
41–45 |
JARパス取得がclasspath依存・Windows依存 |
| 🟡 中 |
170 |
JpegQuality の整数パース |
| 🟡 中 |
426 |
URL判定が startsWith("http") のみ |
| 🟡 低 |
177 |
nobreSize の命名 |
| 🟡 低 |
480 |
出力ファイル名の切り詰め方法 |
特に、今回のコードで実際に修正すべきものは上位3つだと思います。
また、このファイルは以前の AozoraEpub3 のJava版移植・リファクタリングの流れを見ると、元のAozoraEpub3の挙動を100%維持することが重要なので、単純な「綺麗なコードへの書き換え」より、元コードとの差分から挙動が変わった箇所を洗い出すのがかなり有効です。
必要なら次に、**この729行について「元コードの仕様を壊す可能性がある箇所」だけに絞って、行番号付きで徹底的にチェック**できます。
情報源
You:
はい。ここは**「元の coverFileName を変更しない」**ようにするのがポイントです。
現在はファイル処理ループの中で、
Java
if ("0".equals(coverFileName)) {
coverImageIndex=0;
coverFileName="";
} elseif ("1".equals(coverFileName)) {
coverFileName=getSameCoverFileName(srcFile);
}
となっているため、1冊目の処理結果が2冊目に引き継がれてしまいます。
AozoraEpub3
最小限の修正
まず、コマンドラインオプションを読み込んだところで、元の値を別変数に保存します。
例えば現在、
Java
StringcoverFileName="";
としているところを、
Java
StringcoverFileName="";
のままにして、ファイル処理ループに入る前に
Java
StringcoverOption=coverFileName;
を追加します。
そして、ファイル処理ループ内の処理を、
Java
StringcurrentCoverFileName=coverOption;
if ("0".equals(currentCoverFileName)) {
coverImageIndex=0;
currentCoverFileName="";
} elseif ("1".equals(currentCoverFileName)) {
currentCoverFileName=getSameCoverFileName(srcFile);
}
に変更します。
つまり、
Java
if ("0".equals(coverFileName)) {
coverImageIndex=0;
coverFileName="";
} elseif ("1".equals(coverFileName)) {
coverFileName=getSameCoverFileName(srcFile);
}
を丸ごと置き換えます。
ただし、この後で coverFileName を使っている箇所も currentCoverFileName に変更する必要があります。
具体的には
現在の処理が例えば、
Java
StringcoverFileName=commandLine.getOptionValue("c");
...
for (...) {
...
if ("0".equals(coverFileName)) {
coverImageIndex=0;
coverFileName="";
} elseif ("1".equals(coverFileName)) {
coverFileName=getSameCoverFileName(srcFile);
}
...
converter.setCoverFileName(coverFileName);
...
}
という構造なら、
Java
StringcoverFileName=commandLine.getOptionValue("c");
StringcoverOption=coverFileName;
...
for (...) {
...
StringcurrentCoverFileName=coverOption;
if ("0".equals(currentCoverFileName)) {
coverImageIndex=0;
currentCoverFileName="";
} elseif ("1".equals(currentCoverFileName)) {
currentCoverFileName=getSameCoverFileName(srcFile);
}
...
converter.setCoverFileName(currentCoverFileName);
...
}
とします。
なぜこれで直るか
例えば -c 1 で、
book1.txt
book2.txt
book3.txt
を一度に処理した場合です。
現在のコードだと、
最初
coverFileName = "1"
↓ book1
getSameCoverFileName()
↓
coverFileName = "book1.jpg"
↓ book2
"1" ではないので
book2.jpg を探さない
となります。
修正後は、
coverOption = "1"
book1:
currentCoverFileName = "1"
↓
book1.jpg を取得
book2:
currentCoverFileName = "1"
↓
book2.jpg を取得
book3:
currentCoverFileName = "1"
↓
book3.jpg を取得
となります。
さらに重要な点
-c 0 の場合も同じです。
現在は1冊目の処理で
Java
coverFileName="";
に変更されるため、2冊目以降では元の "0" という指定が失われます。
修正後は、
Java
coverOption="0";
を保持して、
Java
StringcurrentCoverFileName=coverOption;
から毎回作るので、各ファイルで独立して処理できます。
要するに、
coverFileName
↓
コマンドラインから指定された「元の設定」として保持
coverOption
↓
元の設定を保持
currentCoverFileName
↓
今回処理している1ファイルだけの結果
という役割分担にするのが最小で安全です。
この修正なら、Java版の既存動作を大きく変えずに、複数入力ファイル時だけ発生する状態引き継ぎバグを直せます。
情報源
ChatGPT:
URI uri = new URI(urlString); url判定方法
You:
URI uri = new URI(urlString); を使うなら、単純な startsWith("http") よりも URIとして正しく解析できるか+schemeを確認する方法が安全です。
AozoraEpub3 の「画像指定がURLかどうか」という用途なら、次の程度が扱いやすいです。
Java
privatebooleanisUrl(StringurlString) {
if (urlString==null||urlString.isEmpty()) {
returnfalse;
}
try {
URIuri=newURI(urlString);
Stringscheme=uri.getScheme();
return"http".equalsIgnoreCase(scheme)
||"https".equalsIgnoreCase(scheme);
} catch (URISyntaxExceptione) {
returnfalse;
}
}
URI と URISyntaxException を使うので、
Java
importjava.net.URI;
importjava.net.URISyntaxException;
を追加します。
例えば
Java
isUrl("https://example.com/cover.jpg") // trueisUrl("http://example.com/cover.jpg") // trueisUrl("HTTPS://example.com/a.jpg") // trueisUrl("cover.jpg") // falseisUrl("C:/book/cover.jpg") // falseisUrl("/home/user/cover.jpg") // falseisUrl("https://") // falseまたはURIとしての妥当性に注意
ただし、「URLらしいか」だけを判定したいのか、「実際に画像URLとして利用可能なURLか」まで判定したいのかで方法が変わります。
AozoraEpub3なら、さらに簡単にするなら
現在おそらく、
Java
if (!coverFileName.startsWith("http")) {
// ローカルファイル
}
のような判定をしている部分だと思います。
ここは、
Java
URIuri=newURI(coverFileName);
Stringscheme=uri.getScheme();
if (!"http".equalsIgnoreCase(scheme)
&&!"https".equalsIgnoreCase(scheme)) {
// ローカルファイル
}
とできます。
ただ、例外処理が必要なので、私は専用メソッド化をおすすめします。
Java
privatebooleanisHttpUrl(Stringvalue) {
if (value==null||value.isEmpty()) {
returnfalse;
}
try {
URIuri=newURI(value);
Stringscheme=uri.getScheme();
return"http".equalsIgnoreCase(scheme)
||"https".equalsIgnoreCase(scheme);
} catch (URISyntaxExceptione) {
returnfalse;
}
}
そして、
Java
if (!isHttpUrl(coverFileName)) {
// ローカルファイルとして処理
}
です。
URI の getScheme() を見るのがポイントです。startsWith("http") だと httpfoo:... のような文字列まで通してしまいます。
ChatGPT:
非 static メソッド 'isHttpUrl(java.lang.String)' は static コンテキストから参照できません
You:
これは isHttpUrl() 自体の問題ではなく、呼び出している場所が static メソッドだからです。
例えば main() は static なので、
Java
publicstaticvoidmain(String[] args) {
if (isHttpUrl(coverFileName)) {
...
}
}
では、インスタンスメソッドの isHttpUrl() を直接呼べません。
今回は static にするのが簡単
main() から使うだけなら、メソッドを
Java
privatestaticbooleanisHttpUrl(Stringvalue) {
if (value==null||value.isEmpty()) {
returnfalse;
}
try {
URIuri=newURI(value);
Stringscheme=uri.getScheme();
return"http".equalsIgnoreCase(scheme)
||"https".equalsIgnoreCase(scheme);
} catch (URISyntaxExceptione) {
returnfalse;
}
}
にしてください。
つまり、
Java
privatebooleanisHttpUrl(Stringvalue)
↓
Java
privatestaticbooleanisHttpUrl(Stringvalue)
だけで解決します。
必要な import は、
Java
importjava.net.URI;
importjava.net.URISyntaxException;
です。
AozoraEpub3.javaの場合
main() 内で、
Java
if (!isHttpUrl(coverFileName)) {
...
}
と呼ぶのであれば、isHttpUrl() を static にするのが最小変更です。
逆に、
Java
privatebooleanisHttpUrl(...)
のままにして、
Java
AozoraEpub3converter=newAozoraEpub3();
converter.isHttpUrl(...)
のようにする必要はありません。
今回の用途では、URL判定は状態を持たない単純なユーティリティなので、static が適しています。