RM-BLOG

IT系技術職のおっさんがIT技術とかライブとか日常とか雑多に語るブログです。* 本ブログに書かれている内容は個人の意見・感想であり、特定の組織に属するものではありません。/All opinions are my own.*

【障害記録】No.11：PDFファイル読込時のバイト欠落※気付かれる前に処理

障害台帳

自分が体験したシステム障害を紹介してトラウマを抉り返す自虐コーナー
※特定避けるため一部脚色・変更しているが、大体ほぼ実体験。

障害No障害分類No
（自分用）発生分類
アプリ/データ/ミドル/ハード案件名個人的ヤバイ度
（５段階評価）

11 B アプリ PDFファイル読込時のバイト欠落
※気付かれる前に処理		 ★★★☆☆

 

 

 

発生日時 2014/02某日 解決日時 2014/02中 発見者 俺 対応者 俺 原因者 俺ではない！（笑）
障害の詳細内容 実はこれはお客さんには気付かれていない案件である。
確実に問題であるのは間違いないのだが、長年誰からも気付かれずに放置されており
あるときふと「これおかしくね？」って気付いて、
しれっと直してリリースしてなかったことにした。
機能特性等から「あまり使われない」という側面もあり、
バグった機能をリリースしてから約７年近くそのまま運用が続けられていた。

複数のPDFファイルをZIPで圧縮してダウンロードできる機能がある。
このとき、内部的には、ZIPのエントリーに各PDFファイルのバイトデータを格納していくのだが、
この「バイトデータ格納」の部分が見事にバグっており、最大250バイトが欠落していた。

下記に簡単にバグっていた実装を抜粋して挙げる。
以下はZIP化については何も記述していない処理例だが、
事象的には同じ（ファイルのバイトデータ読込がバグっている）である。
バグっている例はこちら↓
	private static byte[] readBug(File inFile) throws Throwable {
		byte[] data = new byte[250];
		ByteArrayOutputStream baos = new ByteArrayOutputStream();
		FileInputStream fis = null;
		int readSize = 0;
		try {
			fis = new FileInputStream(inFile);
			while ((readSize = fis.read(data)) >= 0) {
				baos.write(data , 0 , data.length);
			}
		}catch(Throwable e) {
			throw e;
		} finally {
			if (fis != null) {
				fis.close();
			}
		}
		
		return baos.toByteArray();
	
	}

この例では、「バイトデータを読み切る」までの終了条件が
「FileInputStream#readの戻り値が0より大」になっている。
一方で、FileInputStream#readの引数に渡すbyte[]の大きさは250になっている。
このため、例えば元ファイルの容量が550バイトだったりすると、
1回目で250バイト、2回目で500バイト、まで読み込んだ後、
3回目で残りの50バイトを読み込むのだが、
この段階でFileInputStream#readの結果は-1になる（250バイトは読み込めない）ため
ループを抜けてしまい最後の50バイトがByteArrayOutputStreamに格納されない。
結果、読み込んだデータ容量は500バイトになる。

このようにして読み込んだバイトデータをZIPのエントリーに格納していたため、
元ファイルのうち最大250バイトが欠落した状態でZIP化されていたことになる。
実際、使ってみたところ、この機能でダウンロードできるPDFファイル（ZIP解凍後）は、
全てのファイル容量が須らく250の倍数になっていることが確認できた。
すさまじくヤバイ。

と思ったのだが、
元ファイルがPDFというのが幸いしたのか、
それともPDF閲覧にあたって主に利用されていたAdobeがうまい具合に処理していたのか、
どうも末尾の250バイト程度が欠落した程度ではPDFは閲覧に害はないようだった。
実際、オリジナルのPDFファイルと、↑の実装で250バイト欠落状態となったPDFファイルを見比べても、
別になんら遜色はない。
Adobeで開いた時にも特に警告メッセージなどが起きない。
PDFそれ自体が画像ベースかテキストベースか（ないしその両方の組み合わせ）といった点や
PDF化にあたってのツールなどによる違いはあるのだろうが、
基本的にPDFファイルを構成する末尾の250バイト程度は
PDF自体が持つメインの情報とは違う”ダミー”のようなデータにあたるようだ。
バイナリの扱いにはそこまで詳しくないので言及はできないが、
本件の経験的にはそういうことが逆説的に言える。
勿論、上述したように、閲覧ソフト（Adobe等）がいい感じに処理してくれていた可能性はなくはないが…

また、この実装に従うと、
250バイト未満の容量しかないPDFファイルは全て0バイトになってしまうが、
もしそれがあったとするとさすがに確実バレて大問題になっていたことは間違いないが、
運用上250バイト未満のPDFファイルなど存在するわけもなく、
結果的に0バイトPDFファイルが世に見えることはなかった
（というかなんとなくだが、
　中身がどんなにちっぽけであれPDFファイルは250バイトは超えそうな気がする。
　バイナリの構成がどうなってんだか知らないからなんともいえんが…）

この機能が相手にするのが仕様上「PDFファイルだけ」だったというのも救われている。
↑のバグった実装で、例えばCSV等のテキストファイルを相手にした場合、
ダウンロードされるテキストファイルは確実に文字レベルでの欠落（ないし文字化け）が起きるので、一発で問題があるとわかる。
「一見した時バイトデータの欠落が発生したことをユーザが感知できない」
というのをPDFというファイル特性故偶然にも回避していたことが不幸中の幸いであったと言える。
対応内容・経緯等 これに気付いた時、このバグった機能は既に本番運用中であり、
↑に書いたような背景からなのか誰からも指摘受けることなくずーっとそのままだった。
この問題に気づいてからは「速攻で直してなかったことにする」つもりだったが
バイト欠落しても問題起きてないなら別に今のままほっといてもいいんじゃね？って意見も社内ではあがり
実際には修正完了してから少し時間をおいてリリースした。

ちなみに↑のバグったコードを以下のように修正した、
というか
稼働時点からこのメソッドも用意されていたのだが使っていなかった
というのが正直言うと事の真相なのだが。
（まあバグってるメソッドをそのまま残してるのも問題だが…）
	private static byte[] readTruth(File inFile) throws Throwable {
		byte[] data = null;
		ByteArrayOutputStream baos = new ByteArrayOutputStream();
		FileInputStream fis = null;
		int available = 0;
		try {
			fis = new FileInputStream(inFile);
			while ((available = fis.available()) > 0) {
				data = new byte[available];
				baos.write(data , 0 , data.length);
			}
		}catch(Throwable e) {
			throw e;
		} finally {
			if (fis != null) {
				fis.close();
			}
		}
		
		return baos.toByteArray();
	
	}
FileInputStream#availableを使うと、読み込めるバイト容量を取得することができる。
常に250バイトずつ読み込むのではなく、
残読込容量が250をきったら、その分だけ読み込むようにしているのだ。
これにより元ファイルの容量を欠落させることなくそのまま読み込むことができる。
実態を明かせば非常に単純なPGバグであり
（今だから言えることだが）「この程度のこと」すら実装できていなかった、
というのは残念でならない（失笑に値するだろう。。。）
ちなみに一応言っておくけどコーディングしたのは俺ではないからね！！ww
反省点・あとがき こういった案件において、↑に書いたような
「問題起きてないなら別に今のままほっといてもいいんじゃね？」
という意見は、必ず出る。
お客さんが知らない案件のため、その機能を修正する理由がないので、
仮にこの修正がまた別のバグや他機能への影響を発生させ、それがお客さんに知られた場合、
「なんでこんな機能直したんですか？」という疑問が出てくるのは当然で、
そのとき「イヤー実はこの機能バグってたんで直したんすよ！」なんて
馬鹿正直に回答できるわけもない。
一番いいのは（悪くない、というほうが正確か）、
その機能に対して全然別の修正案件があがっていて、
そのときに同時に直してリリースしてしまうことである。
そうすると、仮に「別のバグや他機能への影響」が発生しても、
「全然別の修正案件」のせいにできるからである（その場合でも「デグレ」という話になるのだが）
現実的に運用に支障が出ていないのであれば
「明らかにバグってるけど、下手に手を出して他バグうむよりは、このままそっとしておいたほうが」
という考え方は真っ当な感覚と言える。
ゴルゴ１３も「自分が臆病なため生き残った」と言っている。
まさしくその通りで、臆病に臆病に・慎重に慎重に、という考え方が
物事を成功に導くのである。
→若いころはこの辺の感覚がわからなかったのは苦い思い出としてある。
　非常に軽微な修正を担当した時、「こんな程度の修正でバグるわけがない」と
　自信を持っていたことがある。
　「何をそんなにビビる必要があるんだ？」と。
　しかし年次があがり少なからず責任を負わなければならない立場になっていったとき
　この辺の日和見感覚が出てくるのだ。